refactor(compiler): bump metadata version to 4 #19338
Conversation
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
|
You can preview 034450b at https://pr19338-034450b.ngbuilds.io/. |
chuckjaz
force-pushed
the
bump-metadata-version
branch
from
034450b
to
3c17784
Compare
|
You can preview 3c17784 at https://pr19338-3c17784.ngbuilds.io/. |
chuckjaz
force-pushed
the
bump-metadata-version
branch
2 times, most recently
from
7955baf
to
b5f4cc0
Compare
|
You can preview b5f4cc0 at https://pr19338-b5f4cc0.ngbuilds.io/. |
chuckjaz
force-pushed
the
bump-metadata-version
branch
from
b5f4cc0
to
2762149
Compare
|
You can preview 2762149 at https://pr19338-2762149.ngbuilds.io/. |
chuckjaz
force-pushed
the
bump-metadata-version
branch
from
2762149
to
8da95b9
Compare
|
You can preview 8da95b9 at https://pr19338-8da95b9.ngbuilds.io/. |
| } | ||
| const dtsMetadata = this.getMetadataForSourceFile(dtsFilePath); | ||
| if (dtsMetadata) { | ||
| for (let prop in dtsMetadata.metadata) { |
There was a problem hiding this comment.
newMetadata = {...dtsMetadata, ...newMetadata} ?
There was a problem hiding this comment.
I believe the explicit assignments are more clear and less likely to cause an issue in the future.
| v3Metadata.exports = exports.exports; | ||
|
|
||
| // Only copy exports from exports from metadataa prior to version 3. | ||
| if ((!oldMetadata.version || oldMetadata.version < 3) && dtsMetadata.exports) { |
There was a problem hiding this comment.
Please add docs why. Maybe don't upgrade fesm metadata at all? Or rather, just change their version number, and do nothing otherwise?
There was a problem hiding this comment.
Treating fesm special is insufficient. @angular/cdk explicitly elides the imports to avoid confusion between importing @angular/cdk and importing the sub-entry points. Plus, this shouldn't be fesm specific as it applies to all version 3 metadata, not just fesms.
| expect(hostNestedGenDir.getMetadataFor('metadata_versions/v1_empty.d.ts')).toEqual([ | ||
| {__symbolic: 'module', version: 3, metadata: {}, exports: [{from: './lib/utils'}]} | ||
| ]); | ||
| it(`should upgrade a missing metadata file into v${METADATA_VERSION}`, () => { |
There was a problem hiding this comment.
Maybe add a test for the case of a fesm?
| if (!v3Metadata && v1Metadata) { | ||
| metadatas.push(this.upgradeVersion1Metadata(v1Metadata, dtsFilePath)); | ||
| if (metadatas.length) { | ||
| let maxMetadata: ModuleMetadata = metadatas[0]; |
| let newMetadata: ModuleMetadata = { | ||
| '__symbolic': 'module', | ||
| 'version': METADATA_VERSION, | ||
| 'metadata': {...oldMetadata.metadata}, |
| @@ -15,7 +15,7 @@ | |||
| // semantics of the file in an array. For example, when generating a version 2 file, if version 1 | |||
| // can accurately represent the metadata, generate both version 1 and version 2 in an array. | |||
|
|
|||
| export const VERSION = 3; | |||
| export const METADATA_VERSION = 4; | |||
Also adds auto upgrade from lower version based on the .d.ts file (e.g. from version 3 to 4). This is needed as we are now also capturing type aliases in metadata files (and we rely on this), see 6e3498c.
chuckjaz
force-pushed
the
bump-metadata-version
branch
from
8da95b9
to
6b705d6
Compare
vicb
added
cla: yes
area: core
|
You can preview 6b705d6 at https://pr19338-6b705d6.ngbuilds.io/. |
|
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. |
Also adds auto upgrade from lower version based
on the .d.ts file (e.g. from version 3 to 4).
This is needed as we are now also capturing type aliases
in metadata files (and we rely on this),
see 6e3498c.
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The compiler requires a modified version of metadata but could not detect when metadata met the requirements.
What is the new behavior?
Metadata that meets the compiler requirements are now marked as version 4. Older version of the metadata are upgraded automatically from the corresponding .d.ts.
Does this PR introduce a breaking change?