fix(compiler-cli): libraries compiled with v16.1+ breaking with framework v16.0.x apps #50714
Conversation
This file contains 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
…ar framework v16.0.x If a library is compiling with Angular v16.1.0, the library will break for users that are still on Angular v16.0.x. This happens because the `DirectiveDeclaration` or `ComponentDeclaration` types are not expecting an extra field for `signals` metadata. This field was only added to the generic types in `16.1.0`- so compilations fail with errors like this: ``` Error: node_modules/@angular/material/icon/index.d.ts:204:18 - error TS2707: Generic type 'ɵɵComponentDeclaration' requires between 7 and 9 type arguments. ``` To fix this, we quickly roll back the code for inserting this metadata field. That way, libraries remain compatible with all v16.x framework versions. We continue to include the `signals` metadata if `signals: true` is set. This is not public API anyway right now- so cannot happen- but imagine we expose some signal APIs in e.g. 16.2.x, then we'd need this metadata and can reasonably expect signal-component library users to use a more recent framework core version.
devversion
added
the
area: compiler
devversion
added
action: review
crisbeto
approved these changes
Jun 14, 2023
| // TODO(signals): Always include this metadata starting with v17. Right | ||
| // now Angular v16.0.x does not support this field and library distributions | ||
| // would then be incompatible with v16.0.x framework users. | ||
| if (meta.isSignal) { |
There was a problem hiding this comment.
nit: I'm not sure we'd even need to revert this until we add another parameter after it.
There was a problem hiding this comment.
yeah, although I think making all optional fields required over time is beneficial as we can then introduce required fields in the future, without having to re-order.
devversion
added
action: merge
devversion
added
target: minor
…h Angular framework v16.0.x Update golden
pkozlowski-opensource
requested changes
Jun 14, 2023
There was a problem hiding this comment.
Wouldn't it mean "forward compatibility"? I mean, we don't have guarantees for libraries compiled with 16.1 when it comes to working with 16.0. Things might work by accident, but I don't think we should go out of our ways to support this situation. Doing so would send a precedent / expectations and I'm not sure we want to find ourselves there.
pkozlowski-opensource
approved these changes
Jun 14, 2023
alxhub
approved these changes
Jun 14, 2023
|
This PR was merged into the repository by commit 12bad65. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
thomasturrell
pushed a commit
to thomasturrell/angular
that referenced
this pull request
Aug 29, 2023
…ar framework v16.0.x (angular#50714) If a library is compiling with Angular v16.1.0, the library will break for users that are still on Angular v16.0.x. This happens because the `DirectiveDeclaration` or `ComponentDeclaration` types are not expecting an extra field for `signals` metadata. This field was only added to the generic types in `16.1.0`- so compilations fail with errors like this: ``` Error: node_modules/@angular/material/icon/index.d.ts:204:18 - error TS2707: Generic type 'ɵɵComponentDeclaration' requires between 7 and 9 type arguments. ``` To fix this, we quickly roll back the code for inserting this metadata field. That way, libraries remain compatible with all v16.x framework versions. We continue to include the `signals` metadata if `signals: true` is set. This is not public API anyway right now- so cannot happen- but imagine we expose some signal APIs in e.g. 16.2.x, then we'd need this metadata and can reasonably expect signal-component library users to use a more recent framework core version. PR Close angular#50714
ChellappanRajan
pushed a commit
to ChellappanRajan/angular
that referenced
this pull request
Jan 23, 2024
…ar framework v16.0.x (angular#50714) If a library is compiling with Angular v16.1.0, the library will break for users that are still on Angular v16.0.x. This happens because the `DirectiveDeclaration` or `ComponentDeclaration` types are not expecting an extra field for `signals` metadata. This field was only added to the generic types in `16.1.0`- so compilations fail with errors like this: ``` Error: node_modules/@angular/material/icon/index.d.ts:204:18 - error TS2707: Generic type 'ɵɵComponentDeclaration' requires between 7 and 9 type arguments. ``` To fix this, we quickly roll back the code for inserting this metadata field. That way, libraries remain compatible with all v16.x framework versions. We continue to include the `signals` metadata if `signals: true` is set. This is not public API anyway right now- so cannot happen- but imagine we expose some signal APIs in e.g. 16.2.x, then we'd need this metadata and can reasonably expect signal-component library users to use a more recent framework core version. PR Close angular#50714
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
If a library is compiling with Angular v16.1.0, the library will break for users that are still on Angular v16.0.x. This happens because the
DirectiveDeclarationorComponentDeclarationtypes are not expecting an extra field forsignalsmetadata. This field was only added to the generic types in16.1.0- so compilations fail with errors like this:To fix this, we quickly roll back the code for inserting this metadata field. That way, libraries remain compatible with all v16.x framework versions.
We continue to include the
signalsmetadata ifsignals: trueis set. This is not public API anyway right now- so cannot happen- but imagine we expose some signal APIs in e.g. 16.2.x, then we'd need this metadata and can reasonably expect signal-component library users to use a more recent framework core version.