-
Notifications
You must be signed in to change notification settings - Fork 180
Minify JS extensions by default on app dev #5864
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
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Coverage report
Test suite run success2855 tests passing in 1251 suites. Report generated by 🧪jest coverage report action from 755e25e |
This comment was marked as outdated.
This comment was marked as outdated.
1 similar comment
/snapit |
🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm. Test the snapshot by installing your package globally: pnpm i -g @shopify/[email protected] Tip If you get an Caution After installing, validate the version by running just |
21e66fe
to
755e25e
Compare
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!🚀
Could you please snapshot the version 3.80.4 with this pull request? @gonzaloriestra The 3.80.0 version that your snapshot uses has a different authentication error #5826 that I believe was fixed? |
@keiraarts that fix is already included in this branch (and snapshot). But we are still working on improving it. |
@gonzaloriestra This is still broken on latest for me. |
Sorry to bump this thread, the authentication is not reliable on this version. Every five minutes the CLI requests a re-login. May I request a 3.75.4 Shopify snapshot with minification feature flag? It's the best of both worlds and my team can ship code! It would help us tremendously because the authentication being unreliable (and uncertain if it's because of packet loss on internet connection?) is slowing down our ability to get a big draft extension uploaded. |
@keiraarts could you try with |
WHY are these changes introduced?
Fixes #5825
Fixes https://github.com/Shopify/develop-app-outer-loop/issues/1533
WHAT is this pull request doing?
Enables minification by default on JS extensions while running
app dev
.This is basically the same as @keiraarts suggested in #5831 (thanks!), but adding a flag to ensure we can keep the current behavior, so that users can debug more easily if needed.
How to test your changes?
npm i -g @shopify/[email protected] --@shopify:registry=https://registry.npmjs.org
shopify app init --template none --name minify-app
cd minify-app
shopify app generate extension --template admin_action --name admin-action --flavor typescript
shopify app dev
=> wait until it's ready and quitcat .shopify/dev-bundle/{your-extension-id}/dist/admin-action.js
=> minifiedSHOPIFY_CLI_DISABLE_MINIFICATION_ON_DEV=1 shopify app dev
=> wait until it's ready and quitcat .shopify/dev-bundle/{your-extension-id}/dist/admin-action.js
=> not minifiedMeasuring impact
How do we know this change was effective? Please choose one:
Checklist