-
Notifications
You must be signed in to change notification settings - Fork 757
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
perf(worker-rust): Add dev
env for faster local build times
#5059
Conversation
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7973227046/npm-package-wrangler-5059 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/5059/npm-package-wrangler-5059 Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7973227046/npm-package-wrangler-5059 dev path/to/script.js Additional artifacts:npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7973227046/npm-package-create-cloudflare-5059 --no-auto-update npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7973227046/npm-package-cloudflare-kv-asset-handler-5059 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7973227046/npm-package-miniflare-5059 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7973227046/npm-package-cloudflare-pages-shared-5059 npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/7973227046/npm-package-cloudflare-vitest-pool-workers-5059 Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5059 +/- ##
==========================================
+ Coverage 70.34% 70.38% +0.04%
==========================================
Files 298 298
Lines 15463 15463
Branches 3966 3966
==========================================
+ Hits 10877 10884 +7
+ Misses 4586 4579 -7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for the review! One thing I'm not sure about: Removing the But then again, this template will probably be used with more recent versions of wrangler (since a template is usually used for new projects), so it might just be fine in this case. |
Wrangler 3, when the change to default to local mode occurred, has been out for ~9 months, so personally I wouldn't be too concerned about the change |
Awesome, thanks! :) |
Do we need to update some docs somewhere to match this change? |
One other point I realized: Is I added it originally because it's a very common optimization for wasm use-cases, but maybe that doesn't really count for workers. |
I'm no Rust specialist so I would bow to your (or anyone else's views) but I would note that Workers still have a size limit on what can be deployed. So it might be worth keeping a size optimisation turned on? |
That's a good point. I think optimizing for size by default should be sensible for wasm-builds (like in this case), and users can still change/adapt this later on anyways. So if it's up to me I would keep the |
That seems reasonable to keep IMO |
What this PR solves / how to test:
Add a
dev
-environment towrangler.toml
which builds withworker-build --dev
instead ofworker-build --release
.This makes build times for local development much faster.
Add
opt-level = "s"
toCargo.toml
so release builds are optimized for size.Remove the deprecated
--local
parameter from thewrangler
invocation.Author has addressed the following:
Not sure which of these I should do (and how).