Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rosetta): classes are not correctly identified if package uses an outDir #3225

Merged
merged 13 commits into from
Dec 7, 2021

Conversation

kaizencc
Copy link
Contributor

@kaizencc kaizencc commented Dec 3, 2021

rosetta extract breaks down when a project structure includes outDir or rootDir in the tsconfig file. Given such a project structure, extract may find symbolIds that look like outDir/index:ClassA or rootDir/index:ClassA. Since the jsii assembly is created from the source files in the rootDir, rootDir/index:ClassA is the symbolId we want. So if the outDir is specified, we swap it with the specified rootDir (and if rootDir is not specified, we guess at it the same way node does). This ensures that rosetta extract is structure agnostic.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@kaizencc kaizencc requested a review from rix0rrr December 3, 2021 22:52
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Dec 3, 2021
packages/jsii/lib/symbol-id.ts Outdated Show resolved Hide resolved
packages/jsii/lib/symbol-id.ts Outdated Show resolved Hide resolved
packages/jsii/test/compiler.test.ts Outdated Show resolved Hide resolved
@@ -92,3 +92,85 @@ test('Submodules also have symbol identifiers', async () => {
'index:cookie',
);
});

describe(normalizePath, () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! That is hot damn thorough! 😅

@rix0rrr rix0rrr changed the title feat(rosetta): extract finds fqns of snippets when given an outDir and/or rootDir fix(rosetta): classes are not correctly identified if package uses an outDir Dec 7, 2021
@mergify
Copy link
Contributor

mergify bot commented Dec 7, 2021

The title of this Pull Request does not conform with [Conventional Commits] guidelines. It will need to be adjusted before the PR can be merged.
[Conventional Commits]: https://www.conventionalcommits.org

@rix0rrr
Copy link
Contributor

rix0rrr commented Dec 7, 2021

Oh no, issue on Windows:

FAIL test/symbol-identifiers.test.ts
jsii:   ● normalizePath › specify multiple directories
jsii:     expect(received).toEqual(expected) // deep equality
jsii:     Expected: "root/extra/lib/filename.ts"
jsii:     Received: "root\\extra/lib/filename.ts"
jsii:       164 |       'root/filename.ts',
jsii:       165 |     );
jsii:     > 166 |     expect(normalizePath('out/lib/filename.ts', 'root/extra', 'out')).toEqual(
jsii:           |                                                                       ^
jsii:       167 |       'root/extra/lib/filename.ts',
jsii:       168 |     );
jsii:       169 |     expect(normalizePath('out/lib/filename.ts', '.', 'out/lib')).toEqual(
jsii:       at Object.<anonymous> (test/symbol-identifiers.test.ts:166:71)

@rix0rrr rix0rrr merged commit 05631a7 into main Dec 7, 2021
@rix0rrr rix0rrr deleted the conroy/rootdir branch December 7, 2021 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants