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

--minify/config.minify for scripts #811

Merged
merged 4 commits into from
Apr 16, 2022
Merged

--minify/config.minify for scripts #811

merged 4 commits into from
Apr 16, 2022

Conversation

JacobMGEvans
Copy link
Contributor

@JacobMGEvans JacobMGEvans commented Apr 15, 2022

feat: Added minify as a configuration option and a cli arg, which will minify code for dev and publish

resolves #785

@changeset-bot
Copy link

changeset-bot bot commented Apr 15, 2022

🦋 Changeset detected

Latest commit: 0cfcd36

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

@JacobMGEvans JacobMGEvans force-pushed the minify-script branch 2 times, most recently from 94fd7da to d56c43d Compare April 15, 2022 21:15
@github-actions
Copy link
Contributor

github-actions bot commented Apr 15, 2022

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/2177266299/npm-package-wrangler-811

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/811/npm-package-wrangler-811

Or you can use npx with this latest build directly:

npx https://prerelease-registry.developers.workers.dev/runs/2177266299/npm-package-wrangler-811 dev path/to/script.js

@JacobMGEvans JacobMGEvans force-pushed the minify-script branch 2 times, most recently from f73533a to 14b601c Compare April 15, 2022 21:24
@JacobMGEvans JacobMGEvans added enhancement New feature or request and removed bug labels Apr 15, 2022
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

This is a good start! Left some suggestions.

Comment on lines 1238 to 1241
})
.option("minify", {
describe: "Utilize ESBuild minification on script",
type: "boolean",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
})
.option("minify", {
describe: "Utilize ESBuild minification on script",
type: "boolean",

no need to have this for preview

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't preview just use remote Dev? If we don't have parity with Dev I think it would make more sense to just have it deprecated and give the user a message for the Dev command

Copy link
Contributor

Choose a reason for hiding this comment

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

That's what we currently do, but we don't want to introduce any new flags for this command (we already don't have all the flags from dev)

JacobMGEvans and others added 3 commits April 16, 2022 10:19
…ded to specific environments or inherited from top-level and flags --minify which will minify script for publish and remote Dev.

Benefits:
- The script minification will allow for more code to the edge while not reaching the 1MB limit.
- Obfuscation of code in script.
resolves #785
…ded to specific environments or inherited from top-level and flags --minify which will minify script for publish and remote Dev.

Benefits:
- The script minification will allow for more code to the edge while not reaching the 1MB limit.
- Obfuscation of code in script.
resolves #785
This was bothering me, so just a quick mechanical cleanup. In all the types we define, putting `| undefined` at the end instead of the beginning to be consistent.
@JacobMGEvans JacobMGEvans force-pushed the minify-script branch 2 times, most recently from f68c0ea to 5e2867a Compare April 16, 2022 15:29
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

some last nits

@@ -732,6 +732,10 @@ export async function main(argv: string[]): Promise<void> {
.option("experimental-enable-local-persistence", {
describe: "Enable persistence for this session (only for local mode)",
type: "boolean",
})
.option("minify", {
describe: "minify the script",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
describe: "minify the script",
describe: "Minify the script",

…ded to specific environments or inherited from top-level and flags --minify which will minify script for publish and remote Dev.

Benefits:
- The script minification will allow for more code to the edge while not reaching the 1MB limit.
- Obfuscation of code in script.
resolves #785
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

Looks good!

@threepointone threepointone changed the title Minification for publish & remote scripts --minify/config.minify for scripts Apr 16, 2022
@threepointone threepointone merged commit 8c2c7b7 into main Apr 16, 2022
@threepointone threepointone deleted the minify-script branch April 16, 2022 16:14
@github-actions github-actions bot mentioned this pull request Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: implement minify config in wrangler.toml
2 participants