chore(angular): test schematics and code-splitting by OS-jacobbell · Pull Request #31401 · ionic-team/ionic-framework

OS-jacobbell · 2026-08-24T18:19:16Z

Issue number: internal

What is the new behavior?

New test project for validating Ionic components are split across chunks with the new per-component exports.
Script for testing schematics
Script for automating migration from barrel imports to per-component import.
More exports from the standalone barrel file in package.json.

Does this introduce a breaking change?

Yes
No

vercel · 2026-08-24T18:19:22Z

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

Project	Deployment	Actions	Updated (UTC)
ionic-framework	Ready	Preview	Aug 26, 2026 8:28pm

ShaneK

Glad to see this, it's the test I was after on the exports PR. Two big things I noticed though, the export type move breaks IonicSafeString at runtime, and neither new test fails yet on what it's protecting. There's some other things worth looking at too though, and some nits.

ShaneK · 2026-08-25T17:43:18Z

+    if (baselineChunks.size != 1) {
+      throw new Error("Components should have all been included in the same chunk before migrating.");
+    }
+    if (migratedChunks.size != components.length) {


I don't think this catches the case it's meant to. The set unions both components together, and main.js counts as a chunk, so the count can be 2 while a component sits in the initial bundle.

I tried it by importing IonToggle eagerly into AppComponent. The toggle ends up in main.js, so every landing page visitor downloads it, and this still passes. Checking that the chunk holding ion-toggle isn't one the landing page pulls in would be harder to fool.

The baseline check has the opposite problem, since requiring exactly 1 asserts the broken state is still broken. If esbuild ever splits the barrel on its own, this goes red on an improvement.

What would be a good way to check which chunks the landing page pulls in?

What should it do if esbuild splits the baseline on its own? If that silently starts passing, this test would be asserting nothing.

Built the fixture and confirmed the gap. A healthy migrated build puts the two components in two separate chunks and passes. Importing IonToggle eagerly into AppComponent, so every landing page visitor downloads it, puts the toggle in main.js and leaves the header in its own chunk. Still two chunks, so this stays green.

For what the landing page pulls in, the metafile already has enough. Find the output whose inputs include landing.page.ts, walk its imports transitively, add main.js since that's always loaded, then check the toggle isn't anywhere in that closure while the header is. That fails on the eager case above.

On the baseline, I'd stop asserting on it. Log it for context and let the migrated check carry the test, otherwise the day esbuild starts splitting the barrel on its own this goes red for an improvement.

ShaneK · 2026-08-25T17:43:20Z

+  providers: [
+    { provide: RouteReuseStrategy, useClass: IonicRouteStrategy },
+    provideIonicAngular(),
+    provideRouter(routes, withPreloading(PreloadAllModules), withComponentInputBinding()),


Using PreloadAllModules here undoes what the fixture is meant to show. The router fetches every lazy route after the first navigation, so sitting on the landing page pulls down the home page chunk, toggle included.

Drop withPreloading and rebuild the same source, and the landing page loads its own chunk and leaves the home one alone. So the bundler splits it correctly and the router downloads it all anyway. Could this come out?

I'm not sure if I'm seeing an improvement before vs. after removing withPreloading. What are you looking at to verify it?

You're not seeing it in the build output because there isn't a difference there. The chunks come out the same either way, so what changes is only what the browser fetches.

Loading the landing page without preloading pulls six scripts, and the toggle chunk isn't one of them. With PreloadAllModules put back it pulls seven, the extra one being the chunk that holds the toggle, arriving just after the page settles. So it's the network panel rather than the build output.

Removing it was the right call.

ShaneK · 2026-08-25T17:43:32Z

+  execSync(`npx ng new ${testName} --style css --ssr false --ai-config none`, {cwd: packageRootDir});
+
+  // Install ionic-angular package
+  execSync(`npx ng add --skip-confirmation ../ionic-angular-*`, {cwd: testDir});


The glob doesn't clear old tarballs first the way sync.sh and sync-and-pack.sh both do, and never deletes the one it just made. With 8.8.2, a nightly 9.0.0-dev and 9.0.0 all sitting there the shell hands over 8.8.2 first, and since the CLI takes one collection argument with strict(false), the extras go by silently. A leftover tarball from main means this passes having checked the wrong version.

Exiting 0 is also the only assertion here, and most rules in schematics/add/index.ts no-op rather than throw. With no app.config.ts, addProvideIonicAngular skips quietly, and addIonicStyles writes eleven hardcoded css paths nothing verifies. FW-7692 wants this to fail when schematics files are incorrect. Only missing ones get caught, and running npm run build in the generated project would pick up the css list for free.

Added steps to remove old tarballs and run a build in the generated project.

The build step is a good addition, it picks up the hardcoded css list for free.

That cleanup line doesn't do anything though. It takes a literal path rather than a glob, so with two tarballs sitting there both survive, and there's no cwd either so it resolves against process.cwd() instead of the package root. Same CWD-relative trap as clean.js on the exports PR. So ng add can still end up with more than one tarball on the command line.

The fixture's own sync.sh has the version that works, rm -f *.tgz. Worth deleting the tarball this run creates as well, next to where the test project gets cleaned up.

ShaneK · 2026-08-25T17:43:34Z

+
+function readStatsJson() {
+  if (!fs.existsSync(statsFile)) {
+    throw new Error(`${path.relative(projectDir, statsFile)} was not produced by the ${label} build.`);


Reproduced this one: label belongs to build(), not to this function, so the missing stats path throws a ReferenceError instead of the message. Taking it as an argument and passing 'baseline' and 'migrated' at the two call sites would sort it.

I just removed it from the string; the build type is already announced in another console.log.

ShaneK · 2026-08-25T17:43:35Z

+    "./config": "./dist/common/providers/config.js",
+    "./platform": "./dist/common/providers/platform.js",
+    "./nav-params": "./dist/common/directives/navigation/nav-params.js",
+    "./ion-modal-token": "./dist/common/providers/angular-delegate.js",


The six subpaths above this are bare kebab-case and needed no exclusion. This one takes the ion- prefix but it's a DI token rather than a component, and that's the only reason KNOWN_NON_CORE_ION_COMPONENTS had to be added to the verifier I asked for on the exports PR.

It also points at angular-delegate.js, which exports more than the token. Running the codemod with --print-map sends AngularDelegate and attachView to @ionic/angular/ion-modal-token, so someone's AngularDelegate import gets rewritten to a path named after a modal token.

Calling it ./modal-token matches the siblings and lets the exclusion list go away. Free to change now, breaking once it's released.

I'm confused about this one, AngularDelegate and attachView don't seem to be exported for the standalone components API. Should I make a barrel file that only exports IonModalToken?

The other import paths match the imported class name. It seems confusing to have to import IonModalToken from @ionic/angular/modal-token when everything else would have you expect @ionic/angular/ion-modal-token.

You're right on both counts, and my suggestion was wrong. The ionic-route-strategy entry right below it is kebab-case of the exported name too, so ion-modal-token follows the same rule. And nobody can import AngularDelegate from the barrel anyway, since the main entry resolves to the standalone index which only re-exports the token, so the codemod can't rewrite it in real code even though it turns up in the printed map.

The bit that does still stand is smaller. This subpath publishes angular-delegate.js as an entry point, so AngularFrameworkDelegate, attachView and bindLifecycleEvents become importable from it too, and that's public API once it's released. It's also why the verifier needed the new non-core whitelist. A barrel file exporting just the token, like you suggested, would sort both.

ShaneK · 2026-08-25T17:43:51Z

        name: ionic-angular
        output: ./packages/angular/AngularBuild.zip
        paths: ./packages/angular/dist ./packages/angular/css
+    - name: 📐 Run Angular Package Tests 


Every other package in build.yml keeps the build action to building and archiving, with the suites in their own job. This adds an ng new, two npm installs and two production Angular builds to it.

Since test-angular-e2e needs build-angular, a flaky ng new now skips all five e2e apps and shows up as build-angular failing, and stencil-nightly.yml uses the same action, so an Angular CLI or registry problem would surface as a Stencil nightly failure. It also sits after Check Diff, and both scripts write into the tree.

Would a test-angular-package job with needs: [build-angular] work instead? Tiny nit: trailing space in the step name.

Added an action for test-angular-package that build.yml runs.

ShaneK · 2026-08-25T17:43:52Z

@@ -0,0 +1,18 @@
+import { ComponentFixture, TestBed } from '@angular/core/testing';


Nothing runs these. No workflow touches the app's test target, and test-code-split.js only does npm i, sync.sh and npm run build, so the spec files, test-setup.ts and the vitest and jsdom devDeps are all unreachable. They only assert toBeTruthy() as well, so even wired up they wouldn't cover what this PR protects.

Looks like ng new scaffolding that came along for the ride, so worth either wiring up or dropping.

While you're in there, test-setup.ts explains its matchMedia polyfill with ion-menu and ion-split-pane, and this app uses neither.

Removed all these generated test files.

The files went but the scaffolding around them didn't. The test target in angular.json still lists the deleted setup file in setupFiles, and tsconfig.spec.json still includes it, so npm test in this app now points at a file that isn't there. The ng test script, tsconfig.spec.json and the vitest and jsdom devDependencies can all go with it.

While you're in there, the Capacitor config, ionic.config.json, the environments folder and the production fileReplacements entry are all start-template scaffolding this fixture doesn't use either.

ShaneK · 2026-08-25T17:43:54Z

  'ion-select-popover',
  'ion-slides',
 ];
+const KNOWN_NON_CORE_ION_COMPONENTS  = [


I think this is the FW-7695 part of the PR, but I can't find the rest of it. That ticket asks for the standalone e2e pages to move to per-component imports and for this script to check every export subpath resolves from the barrel too, and the standalone pages are all still on the barrel.

The code-split app does typecheck the entry points for the seven components it uses, which is great, but it's not quite the same coverage. Is FW-7695 meant to be a follow-up?

Nit: double space before the =, and the array's missing a semicolon and trailing comma compared to the one above it. Nothing will catch those since scripts is ignored by eslint and prettier.

I wanted to use the existing test app instead of making a new code-split test, but especially with the lazy-loaded side of the app included it's way too big and complicated.

Since your last review I did add a couple steps to test-angular-e2e that runs the import migration script and re-run the tests.

Maybe the migration script should error if anything that's not a type import didn't get replaced?

Yeah, though I'd scope it narrower than anything unreplaced, since unmapped is legitimately non-empty for symbols with no dedicated entry point. The case that matters is rewriting nothing at all.

Ran into this by accident: pointed the codemod at a build from before this PR's seven new subpaths, and because ionic-route-strategy wasn't among them it left one bare barrel import in main.ts, exited 0, and every Ionic component ended up in main.js. The split was gone and the script reported success. A flag that exits non-zero when zero statements were rewritten, passed in the workflow step, would catch that along with map drift and running from the wrong directory.

Those seven new subpaths turn out to be load-bearing for the whole test, which I hadn't realised.

Separately, nothing in the repo says how to run any of this. The test README covers the build.sh flow and this fixture deliberately sits outside it, so someone hitting a red test-angular-package has nowhere to look.

+    const baselineStats = readStatsJson();
+    const baselineChunks = findChunksForComponents(baselineStats, components);
+
+    execSync(`node ${MIGRATE_IMPORTS_SCRIPT}`, { cwd: PROJECT_DIR, stdio: 'inherit' });


ShaneK

Getting close! The job split and the new build step in the schematics test are both good additions.

Two bigger ones this round. The codemod corrupts any import that has a comment in it, and the code-split assertion still passes when a component ends up in main.js. I built the fixture to check both. There are replies on the older threads too, including a couple where you were right and I wasn't.

ShaneK · 2026-08-26T21:36:18Z

+  return { symbolToEntryPoint, ambiguous };
+}
+
+function splitSpecifiers(text) {


A // comment inside a multi-line import takes the rest of the statement with it, since the whitespace gets collapsed before the split on commas. Feeding it this:

import { IonHeader, // page chrome IonToggle, } from '@ionic/angular';

produces import { // page chrome IonToggle } from '@ionic/angular'; plus a correct separate line for IonHeader. The comment swallows the closing brace and the from clause, so IonToggle disappears and the build dies with TS1003 and friends.

This seems like a big issue since nothing checks the output before writing, so the first sign is a compile error somewhere unrelated. Stripping comments out of the brace body before splitting would cover it, and I think it's worth bailing on the statement rather than rewriting when something unexpected survives.

ShaneK · 2026-08-26T21:36:19Z

+
+function findInstalledIonicAngular(projectRoot) {
+  try {
+    return path.dirname(require.resolve(`${PACKAGE_NAME}/package.json`, { paths: [projectRoot] }));


Despite the header, this never reads the target project. Node's self-reference resolution beats paths here, because the script sits inside a package named @ionic/angular whose exports map includes ./package.json. Passing a directory that doesn't exist still hands back this repo's own manifest, so the walk below never runs. CI doesn't catch it because both call sites point at an app synced from the same build, so the two agree by accident.

Dropping the require.resolve branch and keeping the walk would make the comment true.

Also, scripts/ isn't in files, so this never ships, same limitation as the schematics path on the exports PR. Is it meant to be run by consumers? The header reads like end-user docs.

ShaneK · 2026-08-26T21:36:20Z

+        path: ./packages/angular
+        filename: AngularBuild.zip
+    - name: 🕸️ Install Angular Dependencies
+      run: npm ci


This job pulls down CoreBuild.zip but the schematics test never gets to it. The ng add step installs the packed @ionic/angular, and its @ionic/core dependency is ^9.0.0, so the generated project resolves core from the registry rather than from the PR, 9.0.1 today.

That means the new build step checks this PR's Angular against released core. A PR that adds a core export the Angular types reference fails here with a confusing "has no exported member", and a core change that breaks Angular goes green.

A plain npm run sync won't fix it, since the generated project is its own npm install. I think verify-schematics.js would need to pack core and install that tarball into the test project before the build, roughly what the fixture's sync.sh already does.

ShaneK · 2026-08-26T21:36:29Z

@@ -0,0 +1,26 @@
+name: 'Build Ionic Angular'


Thanks for splitting this out, the job shape looks right now.

Nit: the name and description are still Build Ionic Angular from the action this was copied from, where the siblings are all Test X. The trailing space I mentioned on the old step came across with it. Up to you!

ShaneK · 2026-08-26T21:36:30Z

+      run: node ../../../scripts/migrate-per-component-imports.js
+      shell: bash
+      working-directory: ./packages/angular/test/build/${{ inputs.app }}
+    - name: 🧪 Run Tests


Nit: both test steps have the same name, so a failure doesn't say which import style broke. Maybe tag them "(barrel imports)" and "(per-component imports)"? No worries if you'd rather leave it.

ShaneK · 2026-08-26T21:36:32Z

@@ -0,0 +1,14 @@
+import { bootstrapApplication } from '@angular/platform-browser';
+import { RouteReuseStrategy, provideRouter, withComponentInputBinding, withPreloading, PreloadAllModules } from '@angular/router';


Suggested change

import { RouteReuseStrategy, provideRouter, withComponentInputBinding, withPreloading, PreloadAllModules } from '@angular/router';

import { RouteReuseStrategy, provideRouter, withComponentInputBinding } from '@angular/router';

Nit: looks like withPreloading and PreloadAllModules are left over from before the call came out. Nothing will flag them, since TypeScript elides unused imports and this tree is skipped by eslint.

OS-jacobbell added 3 commits August 21, 2026 10:51

chore(angular): test schematics for ng add

466de9b

add code split test project

d2052cb

add code-split test automation

0ef4de4

github-actions Bot added the package: angular @ionic/angular package label Aug 24, 2026

github-advanced-security Bot found potential problems Aug 24, 2026

View reviewed changes

Comment thread packages/angular/scripts/test-code-split.js Fixed

remove unused svg and update ci test name

5b1ac7f

vercel Bot deployed to Preview August 24, 2026 18:36 View deployment

move angular package tests to ci build-angular step

4c31a83

vercel Bot deployed to Preview August 24, 2026 19:42 View deployment

make standalone type exports explicit

d4d9df9

vercel Bot deployed to Preview August 24, 2026 20:35 View deployment

ShaneK requested changes Aug 25, 2026

View reviewed changes

apply feedback

0ec8487

github-advanced-security Bot found potential problems Aug 26, 2026

View reviewed changes

Comment thread packages/angular/scripts/test-code-split.js

const baselineStats = readStatsJson();

const baselineChunks = findChunksForComponents(baselineStats, components);

execSync(`node ${MIGRATE_IMPORTS_SCRIPT}`, { cwd: PROJECT_DIR, stdio: 'inherit' });

vercel Bot deployed to Preview August 26, 2026 19:34 View deployment

download core archive in test-angular-package action

1f03469

vercel Bot deployed to Preview August 26, 2026 20:28 View deployment

ShaneK requested changes Aug 26, 2026

View reviewed changes

		@@ -0,0 +1,18 @@
		import { ComponentFixture, TestBed } from '@angular/core/testing';

		@@ -0,0 +1,14 @@
		import { bootstrapApplication } from '@angular/platform-browser';
		import { RouteReuseStrategy, provideRouter, withComponentInputBinding, withPreloading, PreloadAllModules } from '@angular/router';

Uh oh!

Conversation

OS-jacobbell commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the new behavior?

Does this introduce a breaking change?

Uh oh!

vercel Bot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ShaneK left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneK left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

OS-jacobbell commented Aug 24, 2026 •

edited

Loading

vercel Bot commented Aug 24, 2026 •

edited

Loading