Skip to content

[ZEPPELIN-6637] Compile Angular decorators in the shell unit test setup - #5435

Open
kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6637
Open

[ZEPPELIN-6637] Compile Angular decorators in the shell unit test setup#5435
kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6637

Conversation

@kimyenac

Copy link
Copy Markdown
Contributor

What is this PR for?

ZEPPELIN-6567 gave the Angular shell a vitest setup, but only for specs that build their subject by hand. The spec it added is named "mounts React remotes outside the Angular zone without TestBed" and constructs the directive with new ReactMountDirective(...). A spec that goes through TestBed does not run yet, for three separate reasons.

Vitest 4 transforms specs with oxc, and that transform does not apply the decorator options the application build uses. They live in tsconfig.base.json, which src/tsconfig.json extends while excluding **/*.spec.ts. A decorated spec therefore fails to parse with SyntaxError: Invalid or unexpected token. Declaring oxc.decorator on the vitest config settles it independently of that lookup.

emitDecoratorMetadata then compiles to a __metadata helper that is a silent no-op unless Reflect.metadata exists, so Angular JIT sees no design:paramtypes and constructor injection fails with NG0202. src/polyfills.ts pairs zone.js with core-js/es7/reflect for the application, and the setup already mirrors the first half, so mirroring the second needs no new dependency.

Finally the setup never calls TestBed.initTestEnvironment(), and with vitest globals disabled Angular cannot install its per-test reset hook, so the second spec in a file hits Cannot configure the test module when the test module has already been instantiated.

One note on the issue description. It attributes all of this to oxc not reading experimentalDecorators and emitDecoratorMetadata, and suggests importing @angular/compiler. The parse failure is real, but the missing polyfill and the missing TestBed initialization are separate causes, and @angular/compiler turns out not to be needed: @angular/core/testing already imports it, so the JIT facade is loaded either way.

The TestBed spec for ReactMountDirective is the first consumer, and sits alongside the hand-driven one rather than replacing it. It renders the directive from a host template, so the decorator metadata, the @Input bindings and the constructor injection all have to resolve for it to run at all. It also provides zone change detection the way main.ts does, without which the zone assertions would pass vacuously against TestBed's zoneless default.

None of the three files ships in the application bundle, so there is no runtime change.

What type of PR is it?

Improvement

Todos

None

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-6637

How should this be tested?

npm run test:shell from zeppelin-web-angular, which is green at 5 specs across 2 files. Each piece of the setup was checked by removing it:

  • the oxc block: the TestBed spec fails to parse, SyntaxError: Invalid or unexpected token
  • core-js/es7/reflect: 3 failures, NG0202: This constructor is not compatible with Angular Dependency Injection
  • initTestEnvironment: Need to call TestBed.initTestEnvironment() first
  • afterEach(resetTestingModule): the second spec onward fails with test module has already been instantiated
  • and to check the new spec is not asserting vacuously, changing the directive's runOutsideAngular to run makes its zone assertion fail

Note that unit tests do not gate CI yet (ZEPPELIN-6566), so this suite has to be run locally for now.

Screenshots (if appropriate)

No, this changes test tooling only.

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

The shell setup can run a spec that constructs a directive by hand, but
not one that goes through TestBed. Three things are missing.

Vitest 4 transforms specs with oxc, and that transform does not apply
the decorator options the application build uses: they live in
tsconfig.base.json, which src/tsconfig.json extends while excluding
**/*.spec.ts. A decorated spec therefore fails to parse with "Invalid or
unexpected token". Declaring the options on the vitest config makes the
transform independent of that lookup.

emitDecoratorMetadata then compiles to a __metadata helper that is a
silent no-op unless Reflect.metadata exists, so constructor injection
fails with NG0202. src/polyfills.ts pairs zone.js with
core-js/es7/reflect for the application, and the setup already mirrors
the first half, so mirroring the second needs no new dependency.

Finally the setup never calls TestBed.initTestEnvironment(), and with
vitest globals disabled Angular cannot install its per-test reset hook,
so the second spec in a file hits "test module has already been
instantiated".

Adds a TestBed spec for ReactMountDirective as the first consumer,
alongside the existing hand-driven one. It renders the directive from a
host template, so the decorator metadata, the @input bindings and the
constructor injection all have to resolve for it to run at all. It also
provides zone change detection the way main.ts does, without which the
zone assertions would pass vacuously against TestBed's zoneless default.

@voidmatcha voidmatcha 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.

LGTM 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants