fix(compiler-cli): libraries compiled with v16.1+ breaking with framework v16.0.x apps#50714
Closed
devversion wants to merge 2 commits intoangular:mainfrom
Closed
fix(compiler-cli): libraries compiled with v16.1+ breaking with framework v16.0.x apps#50714devversion wants to merge 2 commits intoangular:mainfrom
devversion wants to merge 2 commits intoangular:mainfrom
Conversation
…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.
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) { |
Member
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.
Member
Author
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.
…h Angular framework v16.0.x Update golden
pkozlowski-opensource
requested changes
Jun 14, 2023
Member
pkozlowski-opensource
left a comment
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
Member
pkozlowski-opensource
left a comment
There was a problem hiding this comment.
After discussing it further we want to maintain backward compatibility across the same set of major versions.
alxhub
approved these changes
Jun 14, 2023
Member
alxhub
left a comment
There was a problem hiding this comment.
Reviewed-for: global-approvers
Member
|
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. |
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
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.