Skip to content

feat(angular): allow per-component standalone import paths for better code-splitting - #31303

Open
OS-jacobbell wants to merge 12 commits into
major-9.0from
FW-6470
Open

feat(angular): allow per-component standalone import paths for better code-splitting#31303
OS-jacobbell wants to merge 12 commits into
major-9.0from
FW-6470

Conversation

@OS-jacobbell

@OS-jacobbell OS-jacobbell commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

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?

  • Components can be imported from individual files, allowing build tools to bundle some components only with certain pages. E.g. import { IonToolbar } from '@ionic/angular/ion-toolbar'
  • @ionic/angular is now a barrel file, maintaining backwards compatibility.
  • Replaced ng-packagr build system with calling ngc directly.
  • Changed standalone/src pattern to src/standalone.
  • Made packages/angular/package.json the package.json that will be used in the npm package, in line with the React and Vue packages.

Does this introduce a breaking change?

  • Yes
  • No

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.

@OS-jacobbell
OS-jacobbell requested a review from a team as a code owner July 29, 2026 16:48
@OS-jacobbell
OS-jacobbell requested a review from BenOsodrac July 29, 2026 16:48
@vercel

vercel Bot commented Jul 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ionic-framework Ready Ready Preview Jul 31, 2026 2:04pm

Request Review

@github-actions github-actions Bot added package: core @ionic/core package package: angular @ionic/angular package labels Jul 29, 2026

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'});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing shell: true fixes a deprecation warning when building.

@ShaneK ShaneK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

@@ -43,10 +43,119 @@
"validate": "npm i && npm run lint && npm run test && npm run build"
},
"exports": {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This breaks ng add @ionic/angular, and there are two separate causes.

Angular CLI's schematic resolver starts with require.resolve('@ionic/angular/package.json'), which now throws ERR_PACKAGE_PATH_NOT_EXPORTED because the "./package.json" export is gone. The ng-packagr build on major-9.0 generates that entry, so this is a regression.

Then past that, schematics points at ./schematics/collection.json, but build-schematics.js emits into dist/schematics/ and files is ["dist/", "css/"], so the source schematics dir never ships at all. That path only worked before because the publish root was dist/.

I built both branches and ran the CLI's resolver against each: base resolves both steps, this one fails both. We should fix this before merging.

"./ion-checkbox": "./dist/standalone/directives/checkbox.js",
"./ion-datetime": "./dist/standalone/directives/datetime.js",
"./ion-input": "./dist/standalone/directives/input.js",
"./ion-input-otp": "./dist/standalone/directives/inputOtp.js",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"./ion-input-otp": "./dist/standalone/directives/inputOtp.js",
"./ion-input-otp": "./dist/standalone/directives/input-otp.js",

The source file is input-otp.ts, so this subpath resolves to nothing right now. That makes input-otp the only component with no working per-component path.

Couple of others in the same map: "./ion-Tabs" is missing its .js and is capitalized, so ./ion-tabs gives ERR_PACKAGE_PATH_NOT_EXPORTED, and "./ion-Nav" is capitalized too. Everything else here is kebab-case.

Since the ion-*.ts files are generated by Stencil, I think a hand-maintained list this long is going to keep drifting. Would it be worth adding a build step that checks every exports target resolves to an emitted file? That would have caught both of these.

"./toast-controller": "./dist/standalone/providers/toast-controller.js",
"./provide": "./dist/standalone/providers/ionic-angular.js"
},
"files": [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Dropping typings breaks anyone on moduleResolution: node10, since that mode ignores exports entirely. I built the package on major-9.0 to compare: base has typings and resolves fine, this branch gives TS2307 Cannot find module '@ionic/angular'. Both bundler and node16/nodenext are fine.

To be fair, main was never there, so typings is the only real loss. Still, that makes this a breaking change and the PR body has "No" checked. Either add the field back or note it in BREAKING.md and migration docs.

"enableResourceInlining": true,
"compilationMode": "partial"
},
"compilerOptions": {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ten options came out of here: strict, noImplicitAny, noImplicitReturns, noUnusedLocals, noUnusedParameters, forceConsistentCasingInFileNames, lib, target, module, moduleResolution. Was that deliberate?

I tried turning the safety ones back on and the package compiles with zero errors, so nothing here needed them gone. Given that, I'd rather see them stay, because losing strict on a published package means the next person in these files gets no help from the compiler.

Losing forceConsistentCasingInFileNames is a funny one in the same PR that added capitalized export keys.

If some of these did have to go, could you add a comment saying why? The tsconfig.schematics.json does that for its TS 6.0 change, so there's a pattern for it already.

}
],
"declaration": true,
"sourceMap": true,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two things around this line.

Losing importHelpers means __decorate gets inlined into 95 files in dist. I measured it by rebuilding with it restored: about 2.9KB (0.53%) on an 8-component chunk, helper copies going from 18 to 0. Small, but it does cut against what this PR is for. The clearer problem is that tslib is now completely unused, zero references in src or dist, while still being a declared runtime dependency.

Separately, sourceMap is on with no declarationMap, and src isn't in files, so all 188 .map files that ship point at sources that aren't in the tarball. I'm not sure why it's on or what it's actually doing for us here, but we should probably fix it or turn it off if it's not necessary.

@@ -81,7 +81,6 @@ jobs:
version: ${{ inputs.version }}
preid: ${{ inputs.preid }}
working-directory: 'packages/angular'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The paths: packages/angular/dist line just below is missing css. React and Vue both list dist and css, and build-angular/action.yml did get updated for it.

Low stakes since nothing actually downloads this particular archive, the CI path that does consume it is test-angular-e2e and that one's fine. Feel free to ignore, it's just inconsistent with the others.


# Pack @ionic/angular
npm pack ../../../dist
npm pack ../../../

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nothing anywhere in the repo imports a per-component subpath, so the ~106 new public paths have no test coverage at all. I checked them by hand and they typecheck, build, code-split and render fine, but nothing in CI would have caught the two dead entries in the exports map, or catch drift when components get added later.

I think the most useful addition would be a bundle assertion: a page importing only @ionic/angular/ion-toolbar, then assert no other component lands in its chunk. That would cover the actual goal of the PR too, which nothing does right now.

const p = spawn(cmd, args, { cwd: typescriptPath, stdio: 'inherit'});
p.on('close', (code) => {
if (code > 0) {
console.log(`ng-add build exited with ${code}`);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
console.log(`ng-add build exited with ${code}`);
console.log(`schematics build exited with ${code}`);

Leftover from when this was build-core.js. Also clean-generated.js and its npm script are still around but build doesn't call them anymore, so those look like they can go too.

"build": "npm run clean && npm run build.ng && npm run build.css && npm run build.schematics",
"build.css": "node scripts/build-css.js",
"build.schematics": "node scripts/build-schematics.js",
"build.ng": "ngc",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now that ng-packagr is out of the build, the value accessors still carry comments justifying a /*@__PURE__*/ workaround with an ng-packagr issue. There are 11 of them.

Since the comment claims that workaround is what makes tree-shaking work on these components, it'd be good to decide deliberately whether it's still needed rather than leave it stale. If it is still needed for another reason, could you reword it? If not, dropping the workaround would be nice cleanup.

Comment on lines +13 to +14
const typescriptPath = path.join(__dirname, '..', 'node_modules', '.bin');
const p = spawn(cmd, args, { cwd: typescriptPath, stdio: 'inherit'});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const typescriptPath = path.join(__dirname, '..', 'node_modules', '.bin');
const p = spawn(cmd, args, { cwd: typescriptPath, stdio: 'inherit'});
const tsc = require.resolve('typescript/bin/tsc');
const p = spawn(process.execPath, [tsc, ...args], { stdio: 'inherit' });

Saw your note about the deprecation warning, and that's a real one (DEP0190 on Node 22+, args aren't escaped when shell: true). Dropping shell: true is the right call, I think the replacement just needs to go a bit further.

Two things fall out of it as written. Node now resolves tsc from PATH instead of from the cwd option, so cwd pointing at node_modules/.bin doesn't do anything anymore, and it only works because npm puts node_modules/.bin on PATH for you. Running the script outside npm run gives spawn tsc ENOENT. The other is Windows, where tsc is tsc.cmd and Node can't spawn that without a shell, and the Unix .bin/tsc shim isn't executable by Node either. I'm on macOS so I haven't confirmed the Windows half myself, but we do document Windows contribution in CONTRIBUTING.

Spawning process.execPath against TypeScript's JS entry sidesteps all of it, no shell and no deprecation warning. I verified it still builds the schematics. The cmd variable above becomes unused if you go this way.

Separately, the error event isn't handled here, so a spawn failure dies with a raw stack instead of hitting the reject path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: angular @ionic/angular package package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants