feat: batch installs for npm packages #1089
Conversation
🦋 Changeset detectedLatest commit: 7dcae68 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/2385256223/npm-package-wrangler-1089You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1089/npm-package-wrangler-1089Or you can use npx https://prerelease-registry.developers.workers.dev/runs/2385256223/npm-package-wrangler-1089 dev path/to/script.js |
rozenmd
force-pushed
the
batch-npm-calls
branch
4 times, most recently
from
c8b8c73 to
92e8fdf
Compare
rozenmd
force-pushed
the
batch-npm-calls
branch
2 times, most recently
from
8d6b8bb to
7cbd65f
Compare
packages/wrangler/src/index.tsx
Outdated
| await installPackages( | ||
| shouldRunPackageManagerInstall, | ||
| devDepsToInstall | ||
| ); | ||
|
|
There was a problem hiding this comment.
I understand that you are running this here before the writePackageJsonScriptsAndUpdateWranglerToml() function because you want the install to appear before the final instructions to the user.
But I think we could make this cleaner by having an extra "global" called instructions: string[]. Then writePackageJsonScriptsAndUpdateWranglerToml() could add the instructions to that list rather than logging them directly.
This would mean we would just need a single call to installPackages() write at the end, followed by a call to a function like printInstructions() which will then log out the instructions.
There was a problem hiding this comment.
That's a fantastic idea, I'll give that a go now actually
packages/wrangler/src/index.tsx
Outdated
| const devDepsToInstall: string[] = []; | ||
| const instructions: string[] = []; | ||
| let shouldRunPackageManagerInstall = false; | ||
| let hasAlreadyRunPackageInstall = false; |
There was a problem hiding this comment.
Now that we only ever run installPackages() once, this is no longer needed.
Closes #1036