feat(angular): allow per-component standalone import paths for better code-splitting - #31303
feat(angular): allow per-component standalone import paths for better code-splitting#31303OS-jacobbell wants to merge 24 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
package: core
|
|
||
| const p = spawn(cmd, args, { cwd: typescriptPath, stdio: 'inherit', shell: true }); | ||
| const typescriptPath = path.join(__dirname, '..', 'node_modules', '.bin'); | ||
| const p = spawn(cmd, args, { cwd: typescriptPath, stdio: 'inherit'}); |
There was a problem hiding this comment.
Removing shell: true fixes a deprecation warning when building.
There was a problem hiding this comment.
Great work! I built the package and bundled a landing route that imports only IonToolbar: it went from 608KB across 15 chunks, pulling in all 9 components the other route used, down to 47KB across 3 chunks with just ion-toolbar. About a 92% cut for that page. The win comes from code splitting rather than tree-shaking, a whole-app bundle only moves about 0.5%, but this was about code splitting, I'm only highlighting that so people don't get the wrong idea about this PR.
I also ran the ng18 and ng22 test apps and got 189/189 e2e passing on both, and lint and the generated-file diff check are clean.
There are a few things I'd want fixed before this merges though. The big one is ng add @ionic/angular being broken in two separate ways, and anyone on moduleResolution: node10 can't resolve the package at all anymore, which makes this a breaking change which should probably be indicated in the BREAKING.md and docs migration guide despite the checkbox. There's actually already a moduleResolution change indicated in the BREAKING.md and migration guide so this one can just be tacked on to that. There are also two dead entries in the exports map. The rest are smaller and some are just nits.
Co-authored-by: Shane <shane@shanessite.net>
There was a problem hiding this comment.
Great work! Everything from last round is fixed and I re-tested it all.
The code splitting works nicely. A landing route importing only IonToolbar comes out 3 chunks and 46.5 KB with 1 component, against 15 chunks and 605.6 KB with 9 through the barrel.
On the PURE workaround, I checked webpack too since that's what the old comment blamed. Importing one component from the barrel pulls in 0 others, so that worked nicely, good work!
There's still a couple of issues holding this up, but great progress so far!
| "prettier": "@ionic/prettier-config", | ||
| "schematics": "./schematics/collection.json" | ||
| "schematics": "./dist/schematics/collection.json", | ||
| "typings": "./dist/standalone/index.d.ts", |
There was a problem hiding this comment.
The main entry resolves under moduleResolution: node10 now, but the subpaths don't. Importing @ionic/angular/lazy or any per-component path gives TS2307. On major-9.0 all of ., /common and /lazy resolve, because ng-packagr drops a small package.json into each entry-point folder with module and typings in it and node10 follows those. This relies on the exports map alone, which node10 ignores.
I think this is fine to leave, node10 is deprecated in TS 6.0 and Angular 18+ wants bundler. Only mentioning it because the main entry got fixed for the same reason. The common one doesn't matter, the README already says not to import it.
There was a problem hiding this comment.
What's the difference between this for node10 and documentation here? https://github.com/ionic-team/ionic-framework/blob/major-9.0/BREAKING.md?plain=1#L311
There was a problem hiding this comment.
Nope, same thing, that note already covers it. It's more true now than when it was written, if anything, since those per-entry-point shims I mentioned are the bit that's disappearing.
One other thing I noticed while poking around in here: the module field is gone too. Anything that ignores exports and goes off mainFields used to find the root entry through fesm2022, and now it finds nothing at all. React and Vue both set main. You could add "module": "./dist/standalone/index.js" if you want, but honestly I'd put it in the same bucket as the node10 thing and just leave it.
ShaneK
left a comment
ShaneK
left a comment
There was a problem hiding this comment.
Awesome work! Just one actual blocker left
| "prettier": "@ionic/prettier-config", | ||
| "schematics": "./schematics/collection.json" | ||
| "schematics": "./dist/schematics/collection.json", | ||
| "typings": "./dist/standalone/index.d.ts", |
There was a problem hiding this comment.
Nope, same thing, that note already covers it. It's more true now than when it was written, if anything, since those per-entry-point shims I mentioned are the bit that's disappearing.
One other thing I noticed while poking around in here: the module field is gone too. Anything that ignores exports and goes off mainFields used to find the root entry through fesm2022, and now it finds nothing at all. React and Vue both set main. You could add "module": "./dist/standalone/index.js" if you want, but honestly I'd put it in the same bucket as the node10 thing and just leave it.
ShaneK
left a comment
ShaneK
left a comment
There was a problem hiding this comment.
Awesome job on this! 🎉 One nit looks like it just comes from a bad merge resolution, the rest looks good!
Co-authored-by: Shane <shane.king@outsystems.com>
2 participants
Issue number: resolves #30114
What is the current behavior?
Any standalone components imported by one page are bundled with every page. E.g. a landing page will deliver all Ionic Framework components used by the web app, even if most of them are not used by the landing page. Because all components are imported from one file, build tools don't split the code well.
What is the new behavior?
import { IonToolbar } from '@ionic/angular/ion-toolbar'@ionic/angularis now a barrel file, maintaining backwards compatibility.standalone/srcpattern tosrc/standalone.packages/angular/package.jsonthepackage.jsonthat will be used in the npm package, in line with the React and Vue packages.Does this introduce a breaking change?
Other information
I'm unfamiliar with Angular npm package best practices, I just set up what seems reasonable to match the React and Vue ones. Also not sure if release scripts will need to be updated.