fix: dependency submodules may not be discovered #3151
Merged
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
When a dependency exposes submodules, and the entry point of that dependency is never imported by the program being compiled, symbols from the dependency were not correctly mapped to their submodule because the entry point was not part of the compiler input (and hence the `ts.Program` did not hold a `ts.SourceFile` for it, and the `ts.TypeChecker` did not have symbols for exports therein). In order to address this problem, this always explicitly references dependencies' entry point (the `.d.ts` file referenced by the `types` key) to the compiler input path. This guarantees the compiler binds symbols within those, and that the submodule mapper can appropriately discover those. ------------------------------------------------------------------------ An alternative approach was considered (but rejected because it introduced significant complexity): we could have created `ts.SourceFile` objects using `ts.createSourceFile`, which allows us to obtain a set of `ts.Statement` nodes, however those are not bound to symbols and we then have to process the AST without the assistance of the type checker (and would end up having to re-implement a sub-set of the binder/type-checker's features).
rix0rrr
reviewed
Nov 10, 2021
rix0rrr
reviewed
Nov 10, 2021
rix0rrr
approved these changes
Nov 10, 2021
RomainMuller
removed
the
pr/do-not-merge
|
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)... |
|
Merging (with squash)... |
nija-at
pushed a commit
that referenced
this pull request
Nov 15, 2021
This reverts commit 5768bb9. The commit breaks the build of CDKv2 alpha modules. see aws/aws-cdk#17502
mergify bot
pushed a commit
that referenced
this pull request
Nov 16, 2021
…3170) This reverts commit 5768bb9. The commit breaks the build of CDKv2 alpha modules. see aws/aws-cdk#17502 --- 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.
When a dependency exposes submodules, and the entry point of that
dependency is never imported by the program being compiled, symbols
from the dependency were not correctly mapped to their submodule because
the entry point was not part of the compiler input (and hence the
ts.Programdid not hold ats.SourceFilefor it, and thets.TypeCheckerdid not have symbols for exports therein).In order to address this problem, this always explicitly references
dependencies' entry point (the
.d.tsfile referenced by thetypeskey) to the compiler input path. This guarantees the compiler binds
symbols within those, and that the submodule mapper can appropriately
discover those.
An alternative approach was considered (but rejected because it
introduced significant complexity): we could have created
ts.SourceFileobjects usingts.createSourceFile, which allows us toobtain a set of
ts.Statementnodes, however those are not bound tosymbols and we then have to process the AST without the assistance of
the type checker (and would end up having to re-implement a sub-set of
the binder/type-checker's features).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.