feat(compiler): Angular expressions with optional chaining returns undefined#68084
feat(compiler): Angular expressions with optional chaining returns undefined#68084JeanMeche wants to merge 4 commits intoangular:mainfrom
undefined#68084Conversation
f9e97c4 to
a5367d3
Compare
a5367d3 to
b2de89a
Compare
b2de89a to
778f184
Compare
778f184 to
61636b3
Compare
JoostK
left a comment
There was a problem hiding this comment.
Initial round of comments, I haven't gone over the tests nor JIT code yet, and possibly more is pending.
| _experimentalAllowEmitDeclarationOnly?: boolean; | ||
| generateDeepReexports?: boolean; | ||
| generateExtraImportsInLocalMode?: boolean; | ||
| legacyOptionalChaining?: boolean; |
There was a problem hiding this comment.
The LS tsconfig-ng.schema.json would need to be updated.
But, the flag was discussed to be 1P only and therefore not publicly supported, did this change?
There was a problem hiding this comment.
Yes the flag should be 1P only.
This is BazelAndG3Options which isn't public afaik.
There was a problem hiding this comment.
This is BazelAndG3Options which isn't public afaik.
It sorta is, reflected by the public api golden update. It's just its internal interface for grouping that reflects BazelAndG3. Perhaps they aren't meant to be publicly supported, but the current structuring of these options makes them 3P public in my view.
There was a problem hiding this comment.
Yeah I see where you come from.
I don't think we document BazelAndG3Options (beside annotateForClosureCompiler https://angular.dev/reference/configs/angular-compiler-options#annotateforclosurecompiler)
There was a problem hiding this comment.
Considering that @angular/compiler-cli isn't considered public API at all perhaps this is fine if it's not documented, but I do think it's confusing. Perhaps prefixing with an underscore would underpin its not supported status.
|
|
||
| return { | ||
| ...baseMeta, | ||
| legacyOptionalChaining: major < 22 && version !== PLACEHOLDER_VERSION, |
There was a problem hiding this comment.
This is used in more places, but PLACEHOLDER_VERSION is susceptible to problems as it is stamped:
whereas this should always be comparing to the literal 0.0.0-PLACEHOLDER.
Easily avoided using
// Declared using string concatenation to avoid this from being stamped.
const PLACEHOLDER_VERSION = '0.0.0' + '-PLACEHOLDER';perhaps best to address in a separate PR, as this is independent and not necessarily a problem today.
3c53a74 to
07fa535
Compare
JoostK
left a comment
There was a problem hiding this comment.
Few more comments. I am not clear on why the original safe AST nodes aren't used for this, instead of the introduction of isOptional (see one of the comments).
…ndefined` To mitigate this breaking change, this behavior can be disabled by wrapping expressions with the `$null` magic function. : `$null(foo?.bar?.baz)` fixes angular#34385, angular#37622 BREAKING CHANGE: By default optional chainings will now return `undefined` instead of `null`. This behavior can be disabled by wrapping expressions with the `$null` magic function. : `$null(foo?.bar?.baz)`
07fa535 to
dd87d90
Compare
dd87d90 to
466bf4c
Compare
To mitigate this breaking change, this behavior can be disabled by wrapping expressions with the
$safeNavigationMigrationmagic function.:
$safeNavigationMigration(foo?.bar?.baz)fixes #34385, #37622
BREAKING CHANGE: By default optional chainings will now return
undefinedinstead ofnull. This behavior can be disabled by wrapping expressions with the$safeNavigationMigrationmagic function.:
$safeNavigationMigration(foo?.bar?.baz)