Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add hosting config files #440

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

RonithManikonda
Copy link
Collaborator

No description provided.

add configuration files for Vercel, Netlify, and Cloudflare Pages to simplify deployment and address CORS header issues in TutorialKit. This improves the setup experience by automating build commands and headers for common hosting providers, reducing manual configuration steps.

close #234
moved the `generateHostingConfig` function to a new file with improved imports for hosting provider configuration files (`vercel.json`, `netlify.toml`, `_headers`). updated the function to work with the new imports and streamlined configuration generation.

this change reduces clutter in the main implementation file and improves modularity by isolating hosting-related logic.

closes #234
Adjusted the project scaffolding wizard to allow users to select only one hosting provider at a time, laying the groundwork for improved deployment configuration.

Related to #234
Copy link

stackblitz bot commented Feb 5, 2025

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

cloudflare-workers-and-pages bot commented Feb 6, 2025

Deploying tutorialkit-demo-page with  Cloudflare Pages  Cloudflare Pages

Latest commit: 675c5fb
Status: ✅  Deploy successful!
Preview URL: https://404bb26d.tutorialkit-demo-page.pages.dev
Branch Preview URL: https://ronith-add-hosting-config-fi.tutorialkit-demo-page.pages.dev

View logs

@RonithManikonda RonithManikonda changed the title Ronith/add hosting config files feat: add hosting config files Feb 6, 2025
Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start @RonithManikonda! 💯

Some comments below. Also the test are currently stuck as there is no default value for this new option. The tests are using --defaults which skips all prompts and uses default values. You can test this CLI manually by running:

$ cd packages/cli
$ pnpm build
$ node ./dist/index.js create example

# Or to see how tests are using this, see packages/cli/tests/create-tutorial.test.ts
$ node ./dist/index.js create example --no-git --no-install --no-start --defaults

Fixes Prettier errors to pass tests and ensure consistent code style.

close #234
Copy link
Member

@AriPerkkio AriPerkkio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests and build are now passing, nice!

I've added couple of notes below. In addition to those, let's add some test cases in packages/cli/tests/create-tutorial.test.ts. You can run these tests with pnpm run test when in packages/cli directory.

For each provider, add a test case that builds the project and checks that it generated the expected hosting file. Here's and example for Netlify:

test('create a project with Netlify as provider', async (context) => {
  const name = context.task.id;
  const dest = path.join(tmpDir, name);

  await execa('node', [cli, 'create', name, '--no-install', '--no-git', '--defaults', '--provider', 'netlify'], {
    cwd: tmpDir,
  });

  const projectFiles = await fs.readdir(dest, { recursive: true });
  expect(projectFiles).toContain('netlify.toml');
});

And for Cloudflare's case we'll also need to check that package.json contains the postbuild script:

const packageJson = await fs.readFile(`${dest}/package.json`, 'utf8');
const json = JSON.parse(packageJson);

expect(json).toHaveProperty('scripts');
expect(json.scripts).toHaveProperty('postbuild');
expect(json.scripts.postbuild).toBe('cp _headers ./dist/');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants