Add production build for esm-browser #2939
Conversation
Change the rollup config to also build pinia for esm browsers prod It will remove devtools and minify the code Closes vuejs#2205
✅ Deploy Preview for pinia-official ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
I would appreciate it if you could take a look at this PR, @posva. Will be really helpful to be able to use Pinia with Micro-Frontends using importmaps. |
|
Hey @porfirioribeiro! Thanks for the PR, I still haven't got the time to take a look but I wanted to tell you that if you rebase from (or merge) |
WalkthroughUpdates rollup.config.mjs to add a minified browser production build (“browser-prod”), adjust raw ESM detection to include all browser variants, set external dependencies to only 'vue' for browser-prod, and alter output naming to esm-browser.prod.js for the browser format. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesPoem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
rollup.config.mjs (1)
221-237: Terser’smoduleoption is wrong for ESM-browser builds; minification will miss ESM-specific optimizations
module: /^esm/.test(format)returns false for the 'browser' format, so Terser won’t apply ESM-aware minification to the ESM-browser prod build. Switch to a condition that treats 'browser' as a module.- terser({ - module: /^esm/.test(format), + terser({ + module: format === 'browser' || /^esm/.test(format), compress: { ecma: 2015, pure_getters: true, }, }),Optionally bump
compress.ecmato a more recent baseline (e.g., 2020) if your target runtime allows it.
🧹 Nitpick comments (1)
rollup.config.mjs (1)
100-105: Optional: Revisit BUNDLER on the dev ESM-browser buildFor buildName === 'browser', isBundlerESMBuild is true, which sets BUNDLER to true in code. If any runtime branches treat BUNDLER as “runs under a bundler,” this could be misleading for a raw ESM-in-browser build. If intentional/legacy, fine; otherwise, consider restricting isBundlerESMBuild to bundler artifacts (e.g., mjs) only.
If you want to make this change:
- const isBundlerESMBuild = buildName === 'browser' || buildName === 'mjs' + const isBundlerESMBuild = buildName === 'mjs'Note: This affects DEV/TEST/BUNDLER replacements and should be validated against the codebase expectations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
rollup.config.mjs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
🔇 Additional comments (3)
rollup.config.mjs (3)
76-83: Adding a minified ESM-browser build is correct and aligned with the objectiveIncluding the browser format in the minified configs and generating a browser-prod variant is the right call to ship a production-friendly ESM for browsers.
100-105: Correctly classifying both browser and browser-prod as “raw ESM”Switching to buildName.includes('browser') ensures BROWSER applies to both dev and prod browser builds. This is consistent with the intended runtime environment.
160-197: Double-check that devtools code is fully removed from the prod ESM-browser bundleBefore running the check below, make sure you’ve generated the production build (e.g. via
npm run buildor your standard build command) so that thepinia.esm-browser.prod.jsartifact actually exists.Then verify there are no leftover references to
@vue/devtools-apiorvue-demi:# adjust the path to wherever your built files land, e.g. packages/pinia/dist fd -a 'pinia.esm-browser.prod.js' packages/pinia/dist | while read -r f; do echo "Checking $f" rg -n -C1 -e '@vue/devtools-api' -e 'vue-demi' "$f" || true doneIf either string shows up, tree-shaking didn’t eliminate the devtools code—ensure your imports or conditional flags around devtools are fully guard-railed or switched to dynamic imports so Rollup can drop them. Otherwise you’re all set.
| const external = ['vue'] | ||
| if (buildName !== 'browser-prod') external.push('@vue/devtools-api') | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Devtools API is still external in the dev ESM-browser build; this defeats the “only depend on Vue” goal for importmaps
Right now, '@vue/devtools-api' is only inlined for browser-prod. For micro-frontends using importmaps, the dev ESM-browser build should also avoid external imports (other than Vue) to prevent extra importmap entries and match the “only depend on Vue (3)” goal from issue #2205. Inline devtools in the dev ESM-browser build too.
Apply this diff to inline devtools for all browser builds (dev and prod):
- const external = ['vue']
- if (buildName !== 'browser-prod') external.push('@vue/devtools-api')
+ const external = ['vue']
+ // Inline devtools for browser builds (dev and prod) to avoid extra importmaps in MFEs
+ if (!buildName.includes('browser')) external.push('@vue/devtools-api')This keeps devtools external for bundler/Node targets (mjs/cjs) while making both browser variants self-contained wrt devtools. If you prefer to inline devtools only for ESM-browser (and not for global/iife), we can narrow the condition to buildName === 'mjs' || buildName === 'cjs'.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const external = ['vue'] | |
| if (buildName !== 'browser-prod') external.push('@vue/devtools-api') | |
| const external = ['vue'] | |
| // Inline devtools for browser builds (dev and prod) to avoid extra importmaps in MFEs | |
| if (!buildName.includes('browser')) external.push('@vue/devtools-api') |
🤖 Prompt for AI Agents
In rollup.config.mjs around lines 128 to 130, '@vue/devtools-api' is only
inlined for buildName === 'browser-prod', which leaves the dev ESM-browser build
depending on an external devtools package; change the condition so devtools
remains external only for bundler/Node targets (mjs/cjs) and is inlined for
browser builds. Concretely, replace the current conditional with one that pushes
'@vue/devtools-api' to external only when buildName === 'mjs' || buildName ===
'cjs' (or equivalent negation of browser builds), ensuring both browser dev and
prod builds are self-contained while mjs/cjs keep devtools external.
Thanks @posva, whenever you have time take a look at this. The temporarily deployed URLs will be helpful for testing. But would be nice to have this official so it could be part of the other npm CDNs Thank you |
commit: |
Change the rollup config to also build pinia for esm browsers prod It will remove devtools and minify the code
Closes #2205
Summary by CodeRabbit
New Features
Chores