fix(hydrate): prevent dead code elimination of patch dom implementation

[sean-perkins]

What is the current behavior?

In certain environments, such as Angular Universal (with Angular 16 or newer), the baseURI patch code will be eliminated from the final bundle. Javascript optimizers will treat the code as deadcode and eliminate it.

This causes issues in Ionic Framework: ionic-team/ionic-framework#28077

GitHub Issue Number: N/A

What is the new behavior?

• baseURI patched implementation is assigned, so that it is not eliminated from the bundle.
• Ionic Framework (v7) works with Angular Universal with Angular 16 or newer

Does this introduce a breaking change?

• Yes
• No

Testing

• Ryan provided a dev-build of this change for Stencil (4.4.1-dev.1697216096.18bf460)
• Ionic Framework was updated with the dev-build and a new dev-build was created/posted here: bug: IonicServerModule throws runtime error in Angular 16 with SSR enabled ionic-team/ionic-framework#28077 (comment)
• Installed the dev-build in the user's test application and confirmed the application renders
  • Additionally verified that the baseURI code ends up in the final bundle

Other information

This guidance was provided directly by the Angular team.

[github-actions]

--strictNullChecks error report

Typechecking with --strictNullChecks resulted in 1367 errors on this branch.

That's the same number of errors on main, so at least we're not creating new ones!

reports and statistics

Our most error-prone files

Path	Error Count
src/dev-server/index.ts	37
src/mock-doc/serialize-node.ts	36
src/dev-server/server-process.ts	32
src/compiler/build/build-stats.ts	23
src/compiler/style/test/optimize-css.spec.ts	23
src/testing/puppeteer/puppeteer-element.ts	23
src/compiler/output-targets/dist-lazy/generate-lazy-module.ts	22
src/compiler/prerender/prerender-main.ts	22
src/runtime/client-hydrate.ts	19
src/screenshot/connector-base.ts	19
src/runtime/vdom/vdom-render.ts	18
src/compiler/config/test/validate-paths.spec.ts	16
src/dev-server/request-handler.ts	15
src/compiler/prerender/prerender-optimize.ts	14
src/compiler/sys/stencil-sys.ts	14
src/compiler/transpile/transpile-module.ts	14
src/runtime/vdom/vdom-annotations.ts	14
src/sys/node/node-sys.ts	14
src/compiler/build/build-finish.ts	13
src/compiler/prerender/prerender-queue.ts	13

Our most common errors

Typescript Error Code	Count
TS2345	403
TS2322	384
TS18048	310
TS18047	100
TS2722	38
TS2532	34
TS2531	23
TS2454	14
TS2352	13
TS2769	10
TS2790	10
TS2538	8
TS2416	6
TS2344	5
TS2493	3
TS2488	2
TS18046	2
TS2684	1
TS2430	1

Unused exports report

There are 14 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!

Unused exports

File	Line	Identifier
src/runtime/bootstrap-lazy.ts	21	setNonce
src/screenshot/screenshot-fs.ts	18	readScreenshotData
src/testing/testing-utils.ts	198	withSilentWarn
src/utils/index.ts	145	CUSTOM
src/utils/index.ts	269	normalize
src/utils/index.ts	7	escapeRegExpSpecialCharacters
src/compiler/app-core/app-data.ts	25	BUILD
src/compiler/app-core/app-data.ts	115	Env
src/compiler/app-core/app-data.ts	117	NAMESPACE
src/compiler/fs-watch/fs-watch-rebuild.ts	123	updateCacheFromRebuild
src/compiler/types/validate-primary-package-output-target.ts	61	satisfies
src/compiler/types/validate-primary-package-output-target.ts	61	Record
src/testing/puppeteer/puppeteer-declarations.ts	485	WaitForEventOptions
src/compiler/sys/fetch/write-fetch-success.ts	7	writeFetchSuccessSync

[sean-perkins]

Let me know if we have any concerns to this approach. Happy to run through the Angular Universal testing I did locally. I also have additional comments to the issue in the linked framework issue.

[rwaskiewicz]

@sean-perkins Thanks!

Can you expound on

Installed the dev-build in the user's test application and confirmed the application renders
Additionally verified that the baseURI code ends up in the final bundle

and the steps you took to do that? Ideally, if we were to be going through git blame in the future and wanted to figure out how this was tested, it'd be ideal if there were steps that someone on the team could use to reproduce the error/fix

[rwaskiewicz]

Do we need to assign to win here to avoid dead code elimination? I.E. would something like:

Suggested change

	win.__stencil_baseURI = doc.baseURI;
	const __stencil_baseURI = doc.baseURI;

(with maybe an eslint disable rule pragma above it for unused variables) prevent us from elimination of this code without adding something to global?

[sean-perkins]

Test reproduction steps would follow the feedback here: ionic-team/ionic-framework#28077 (comment)

Only with the modifications of:

1. Install the provided Ionic dev-build
2. Run against Angular 16 (the issue reporters originally reproduction app)
3. Validate that:

try {
  win.__stencil_baseURI = e2.baseURI;
} catch {
  Object.defineProperty(e2, "baseURI", {
    get() {
      const t3 = e2.querySelector("base[href]");
      return t3
        ? new URL(t3.getAttribute("href"), r.location.href).href
        : r.location.href;
    },
  });
}

is in the patchDomImplementation function in the compiled bundle.

I do not know if a local assignment of an unused constant would get treated as a side-effect. The Angular team was not certain to which underlying package update between Angular 15 → Angular 16 had an impact on the analysis and removal of dead code. The recommendation from them was assigning to win after looking at Stencil's implementation, but if there is some performance impact to assigning to mock window we can discuss that.

[rwaskiewicz]

Okay, that all sounds reasonable to me - can you help me understand where I'd find that code snippet? So far, I've:

1. Downloaded the reproduction case linked above
2. Installed your dev build
3. Verified we're using Angular 16
4. Run npm run build, but I'm not sure where I should be looking for patchDomImplementation. I'm sure I'm missing something, but I'm not sure what exactly.

For context, I put together a branch of Stencil with the suggested change to try it out, created a dev build of Stencil and then used that to create a dev build of Framework. I wanted to try this out myself, but first want to verify that that code snippet you provided with the existing dev build (so I could tell if/what my build changed)

[SebastianKohler]

Hope I'm not stepping on any toes pitching in ... (in that case, apologies).

Run the command npm run build:ssr to build the SSR app. Open the generated dist/server/main.js, optionally format it, and look for function patchDomImplementation in it.

I can find the code snippet, with one difference. I have r.__stencil_baseURI=e2.baseURI instead of win.__stencil_baseURI=e2.baseURI

[sean-perkins]

Apologies for the delay testing this, I created a new build with your dev-build for using a const: 7.5.6-dev.11700071366.1395b143 (Framework dev-build).

This does not resolve the problem. Bundler still optimizes that bit of code for removal.