fix: answer DOM probes on NativeScriptDocument instead of throwing#176
Open
edusperoni wants to merge 1 commit into
Open
fix: answer DOM probes on NativeScriptDocument instead of throwing#176edusperoni wants to merge 1 commit into
edusperoni wants to merge 1 commit into
Conversation
* Angular discovers some optional configuration by reading the DOM. The
CSP_NONCE default factory, for example, evaluates
`DOCUMENT.body.querySelector('[ngCspNonce]')`. `NativeScriptDocument.body`
was a hollow object, so the optional chaining in those probes did not help
and they failed with `body?.querySelector is not a function`. Signal forms
reach this on the first `[formField]` binding, which crashed every
signal-form dialog at construction.
* `querySelector`, `querySelectorAll`, `getElementById`,
`getElementsByTagName` and `getElementsByClassName` now answer "nothing
found" on the document and on `body`/`head`. `createElement` still throws,
since constructing real nodes is a caller bug rather than a probe.
* CSP_NONCE is also provided as `null`, as there is no CSP in a NativeScript
runtime. It has to be a root provider: the root injector resolves
`providedIn: 'root'` tokens itself before consulting the platform injector,
so providing it in COMMON_PROVIDERS would have had no effect.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Angular discovers some optional configuration by reading the DOM.
NativeScriptDocumentexposed abodythat was truthy but hollow ({ isOverride: true }), so those probes did not fall into their "not found" path — they threw.The clearest case is
CSP_NONCE, whose default factory is:The
body?.guard anticipates a missingbody, not abodywithoutquerySelector, so this fails withinject(...).body?.querySelector is not a function.Signal forms hit this on the very first
[formField]binding:FormFieldinjects the rootInputValidityMonitor, whose default implementation (AnimationInputValidityMonitor) injectsCSP_NONCEin a field initializer. The result is that every signal-form dialog crashes at construction on Angular 22. Note thatinject(CSP_NONCE, { optional: true })does not help —InjectionTokendefaults toprovidedIn: 'root', so the token always resolves and the DOM-reading factory always runs.Changes
NativeScriptDocumentnow answers DOM lookups with "nothing found" rather than leaving the methods undefined:querySelector,querySelectorAll,getElementById,getElementsByTagName,getElementsByClassNameon the document itself, plusbodyandhead(getAttribute/hasAttributeincluded there).createElementdeliberately keeps throwing. Probing for optional config should degrade quietly; constructing real nodes is a caller bug and should stay loud.CSP_NONCEis provided asnullinNATIVESCRIPT_MODULE_STATIC_PROVIDERS. There is no CSP in a NativeScript runtime, and providing it skips the DOM-reading factory entirely.Why
CSP_NONCEmust be a root providerIt is deliberately in
NATIVESCRIPT_MODULE_STATIC_PROVIDERS(root scope) rather thanCOMMON_PROVIDERS(platform scope).R3Injector.get()checks its own records, then self-instantiates anyprovidedIn: 'root'token whose scope it matches, and only then delegates to its parent. A platform-level provider is therefore never consulted. Verified empirically — see below.Verification
Ran Angular 22's real DI against both the old and new document shapes:
body, no provider (status quo)inject(...).body?.querySelector is not a functionnullCSP_NONCEprovider onlynullnullCSP_NONCEproviderThe first row reproduces the reported crash string exactly.
nx build angularpasses, andpackages/angular/src/lib/platform-nativescript.tsis prettier-clean.nativescript.tshas pre-existing formatting drift that I left alone to keep the diff focused.Follow-up
This is the robust workaround. The cleaner fix belongs upstream:
InputValidityMonitoris an abstract class whose provider is{ providedIn: 'root', useClass: forwardRef(() => AnimationInputValidityMonitor) }— an obvious platform seam that Angular's own tests already override — but the symbol is not exported. I have a companion Angular PR addingɵInputValidityMonitorto@angular/forms/signals. Once that lands,@nativescript/angularcan provide an inert monitor and never touch the DOM at all.