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: enhance wrangler init #304

Merged
merged 1 commit into from
Jan 26, 2022
Merged

feat: enhance wrangler init #304

merged 1 commit into from
Jan 26, 2022

Conversation

threepointone
Copy link
Contributor

This PR adds some enhancements/fixes to the wrangler init command.

  • doesn't overwrite wrangler.toml if it already exists
  • installs wrangler when creating package.json
  • offers to install wrangler into package.json even if package.json already exists
  • offers to install @cloudflare/workers-types even if tsconfig.json already exists
  • pipes stdio back to the terminal so there's feedback when it's installing npm packages

This does have the side effect of making our tests a little slower. I added --prefer-offline to the npm install calls to make this a shade quicker, but I can't figure out a good way of mocking these. I'll think about it some more later. We should work on making the installs themselves quicker (re: #66)

This PR also fixes an issue with our tests: runWrangler() would catch thrown errors, and if we didn't manually verify the error, tests would pass. Instead, it now throws correctly, and I modified all the tests to assert on thrown errors. It seems like a lot, but it was just mechanical rewriting.

@changeset-bot
Copy link

changeset-bot bot commented Jan 25, 2022

🦋 Changeset detected

Latest commit: 43ebbec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
wrangler Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@threepointone threepointone force-pushed the enhance-wrangler-init branch 2 times, most recently from fa750f9 to 88395bb Compare January 25, 2022 18:29
@threepointone
Copy link
Contributor Author

Not bad, it didn't get that much slower.

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Nice work @threepointone! I love the new test helper.
If you can resolve all my questions then I will happily mark this as approved.

packages/wrangler/src/__tests__/index.test.ts Outdated Show resolved Hide resolved
packages/wrangler/src/__tests__/jest.setup.ts Outdated Show resolved Hide resolved
packages/wrangler/src/__tests__/publish.test.ts Outdated Show resolved Hide resolved
packages/wrangler/src/__tests__/index.test.ts Outdated Show resolved Hide resolved
packages/wrangler/src/__tests__/index.test.ts Show resolved Hide resolved
packages/wrangler/src/__tests__/index.test.ts Outdated Show resolved Hide resolved
packages/wrangler/src/index.tsx Show resolved Hide resolved
packages/wrangler/src/index.tsx Show resolved Hide resolved
@threepointone
Copy link
Contributor Author

Answered, modified, parried. Looks good?

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. LGTM with potential for follow ups later.

packages/wrangler/src/__tests__/index.test.ts Show resolved Hide resolved
packages/wrangler/src/__tests__/index.test.ts Outdated Show resolved Hide resolved
@threepointone threepointone force-pushed the enhance-wrangler-init branch 2 times, most recently from a5afdae to d8fc913 Compare January 26, 2022 10:41
This PR adds some enhancements/fixes to the `wrangler init` command.

- doesn't overwrite `wrangler.toml` if it already exists
- installs `wrangler` when creating `package.json`
- offers to install `wrangler` into `package.json` even if `package.json` already exists
- offers to install `@cloudflare/workers-types` even if `tsconfig.json` already exists
- pipes stdio back to the terminal so there's feedback when it's installing npm packages

This does have the side effect of making our tests a _little_ slower. I added `--prefer-offline` to the `npm install` calls to make this a shade quicker, but I can't figure out a good way of mocking these. I'll think about it some more later. We should work on making the installs themselves quicker (re: #66)

This PR also fixes an issue with our tests: `runWrangler()` would catch thrown errors, and if we didn't manually verify the error, tests would pass. Instead, it now throws correctly, and I modified all the tests to assert on thrown errors. It seems like a lot, but it was just mechanical rewriting.
@threepointone threepointone merged commit 7477b52 into main Jan 26, 2022
@threepointone threepointone deleted the enhance-wrangler-init branch January 26, 2022 10:48
@github-actions github-actions bot mentioned this pull request Jan 26, 2022
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