fix(jsii): correctly identify types regardless of import method #3233
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
|
This PR is currently blocked on #3225, as the build of |
njlynch
reviewed
Dec 7, 2021
Yessir! Coming up. I got sidetracked here... it's a little annoying because I had the test already, but apparently I didn't commit it so it must have gotten lost in a |
Types imported from dependency's submodules are only correctly
identified if they are imported via a top-level namespace import.
Otherwise, they are incorrectly identified as coming from the top-level
scope and compilation will fail.
Compare:
```ts
// Works
import { aws_ec2 as ec2 } from 'aws-cdk-lib';
// Does not work
import * as ec2 from 'aws-cdk-lib/aws-ec2';
```
Fortunately we already have symbol ids to solve this exact issue
for Rosetta, so we can resort to looking up FQN by symbol id.
Fixes aws/aws-cdk#17860.
rix0rrr
force-pushed
the
huijbers/a-symbol-by-any-other-import
branch
from
5b7ff0d to
73d3779
Compare
njlynch
approved these changes
Dec 8, 2021
|
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
|
Merging (with squash)... |
rix0rrr
added a commit
that referenced
this pull request
Dec 10, 2021
The fix introduced in #3233 relied on the jsii libraries being recompiled with symbolid information. This information was introduced in jsii 1.39.0. With the code from #3233, symbols from libraries compiled with older jsii versions could not be located and the compilation would fail. In this PR, restore the compatibility by falling back to the same FQN that the original code would have guessed. It will not be correct if the type comes from a submodule, but only CDK is using submodules and CDK is using a new jsii compiler.
rix0rrr
added a commit
that referenced
this pull request
Dec 10, 2021
The fix introduced in #3233 relied on the jsii libraries being recompiled with symbolid information. This information was introduced in jsii 1.39.0. With the code from #3233, symbols from libraries compiled with older jsii versions could not be located and the compilation would fail. In this PR, restore the compatibility by falling back to the same FQN that the original code would have guessed. It will not be correct if the type comes from a submodule, but only CDK is using submodules and CDK is using a new jsii compiler.
mergify bot
pushed a commit
that referenced
this pull request
Dec 10, 2021
The fix introduced in #3233 relied on the jsii libraries being recompiled with symbolid information. This information was introduced in jsii 1.39.0. With the code from #3233, symbols from libraries compiled with older jsii versions could not be located and the compilation would fail. In this PR, restore the compatibility by falling back to the same FQN that the original code would have guessed. It will not be correct if the type comes from a submodule, but only CDK is using submodules and CDK is using a new jsii compiler. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
rix0rrr
added a commit
that referenced
this pull request
Dec 29, 2021
When trying to use submodule types from assemblies compiled with a jsii version `< 1.39.0`, you will see the following error: ``` Unable to resolve type "<module>.<type>". It may be @internal or not exported from the module's entry point (as configured in "package.json" as "main"). ``` The type resolution introduced in #3233 was relying on either (a) the symbol identifier table; or (b) the type not being in a submodule. Unfortunately, there was a 3rd case which was not covered: the library had already been imported by its entry point, and the `this.submoduleMap` had been initialized will all exported submodules. Restore that code path, and improve the error message for undeclared dependencies.
mergify bot
pushed a commit
that referenced
this pull request
Dec 30, 2021
…3306) When trying to use submodule types from assemblies compiled with a jsii version `< 1.39.0`, you will see the following error: ``` Unable to resolve type "<module>.<type>". It may be @internal or not exported from the module's entry point (as configured in "package.json" as "main"). ``` The type resolution introduced in #3233 was relying on either (a) the symbol identifier table; or (b) the type not being in a submodule. Unfortunately, there was a 3rd case which was not covered: the library had already been imported by its entry point, and the `this.submoduleMap` had been initialized will all exported submodules. Restore that code path, and improve the error message for undeclared dependencies. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
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.
Types imported from dependency's submodules are only correctly
identified if they are imported via a top-level namespace import.
Otherwise, they are incorrectly identified as coming from the top-level
scope and compilation will fail.
Compare:
Fortunately we already have symbol ids to solve this exact issue
for Rosetta, so we can resort to looking up FQN by symbol id.
Fixes aws/aws-cdk#17860.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.