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.

Sign up for GitHub

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

Jump to bottom

Unhandled error when computing local module specifier #37731

Closed
devversion opened this issue Apr 1, 2020 · 1 comment
Closed

Unhandled error when computing local module specifier #37731

devversion opened this issue Apr 1, 2020 · 1 comment
Assignees
@sheetalkamat
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Milestone
TypeScript 4.0.1

Comments

@devversion
Copy link
Contributor

TypeScript Version: 3.8.3

Search Terms:

  • TypeError: Cannot read property 'lastIndexOf' of undefined
  • getPathRelativeToRootDirs throws due to missing undefined guard.
  • Type Checker throws if non-existent symbols are resolved, but rootDirs are not absolute.

Code

An easy reproduction can be found here: https://github.com/devversion/ts-checker-root-dir-runtime-exception.

Steps:

  1. Clone the repo
  2. Run yarn
  3. Run node ./test.js.
  4. Check the console output for the exception:
TypeError: Cannot read property 'lastIndexOf' of undefined
    at Object.startsWith (<..>\node_modules\typescript\lib\typescript.js:2044:20)
    at isPathRelativeToParent (<..>\node_modules\typescript\lib\typescript.js:102733:23)
    at <..>/node_modules\typescript\lib\typescript.js:102692:24
    at Object.firstDefined (<..>\node_modules\typescript\lib\typescript.js:347:26)
    at getPathRelativeToRootDirs (<..>\node_modules\typescript\lib\typescript.js:102690:23)

Explanation on what is happening:

Currently when developers parse a tsconfig using ts.parseJsonConfigFileContent, a base path needs to be specified. If that base path is not an absolute file path, the type checker can fail
down the road when it tries to compute relative paths based on rootDirs.

This can be observed when the relative tsconfig path is made absolute in the test.js file. At this point, the type checker will not throw, and getSymbolAtLocation works as expected.

Expected behavior:

Calling typeChecker.getSymbolAtLocation (as seen in test.js) should never cause a runtime exception. Hence getPathRelativeToRootDirs should be updated to catch cases where the relative path cannot be computed (i.e. undefined is returned).

Similarly, I'd expect ts.parseJsonConfigFileContent() to either throw if no absolute base path is specified, or expect the path to be resolved based on the CWD. Not sure if that makes sense though.

Actual behavior:

It throws with a runtime exception. See error snippet above. The error surfaces because no relative path can be computed in getPathRelativeToRootDirs. This is because (as explained above), the root directories are not absolute paths, and getPathRelativeToRootDirs does not guard against undefined being returned from getRelativePathIfInDirectory.

Related Issues:

devversion added a commit to devversion/angular that referenced this issue Apr 1, 2020
@devversion
fix(core): avoid migration error when non-existent symbol is imported
6573756 
In rare cases a project with configured `rootDirs` that has imports to
non-existent identifiers could fail in the migration.

This happens because based on the application code, the migration could
end up trying to resolve the `ts.Symbol` of such non-existent
identifiers. This isn't a problem usually, but due to a upstream bug
in the TypeScript compiler, a runtime error is thrown.

This is because TypeScript is unable to compute a relative path from the
originating source file to the imported source file which _should_
provide the non-existent identifier. An issue for this has been reported
upstream: microsoft/TypeScript#37731. The
issue only surfaces since our migrations don't provide an absolute base
path that is used for resolving the root directories.

To fix this, we ensure that we never use relative paths when parsing
tsconfig files. More details can be found in the TS issue.

Fixes angular#36346.
@devversion devversion mentioned this issue Apr 1, 2020
Closed
@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Apr 1, 2020
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.0 milestone Apr 1, 2020
devversion added a commit to devversion/angular that referenced this issue Apr 2, 2020
@devversion
fix(core): avoid migration error when non-existent symbol is imported
e5ce569 
In rare cases a project with configured `rootDirs` that has imports to
non-existent identifiers could fail in the migration.

This happens because based on the application code, the migration could
end up trying to resolve the `ts.Symbol` of such non-existent
identifiers. This isn't a problem usually, but due to a upstream bug
in the TypeScript compiler, a runtime error is thrown.

This is because TypeScript is unable to compute a relative path from the
originating source file to the imported source file which _should_
provide the non-existent identifier. An issue for this has been reported
upstream: microsoft/TypeScript#37731. The
issue only surfaces since our migrations don't provide an absolute base
path that is used for resolving the root directories.

To fix this, we ensure that we never use relative paths when parsing
tsconfig files. More details can be found in the TS issue.

Fixes angular#36346.
devversion added a commit to devversion/angular that referenced this issue Apr 2, 2020
@devversion
fix(core): avoid migration error when non-existent symbol is imported
f838c26 
In rare cases a project with configured `rootDirs` that has imports to
non-existent identifiers could fail in the migration.

This happens because based on the application code, the migration could
end up trying to resolve the `ts.Symbol` of such non-existent
identifiers. This isn't a problem usually, but due to a upstream bug
in the TypeScript compiler, a runtime error is thrown.

This is because TypeScript is unable to compute a relative path from the
originating source file to the imported source file which _should_
provide the non-existent identifier. An issue for this has been reported
upstream: microsoft/TypeScript#37731. The
issue only surfaces since our migrations don't provide an absolute base
path that is used for resolving the root directories.

To fix this, we ensure that we never use relative paths when parsing
tsconfig files. More details can be found in the TS issue.

Fixes angular#36346.
devversion added a commit to devversion/angular that referenced this issue Apr 3, 2020
@devversion
fix(core): avoid migration error when non-existent symbol is imported
9101561 
In rare cases a project with configured `rootDirs` that has imports to
non-existent identifiers could fail in the migration.

This happens because based on the application code, the migration could
end up trying to resolve the `ts.Symbol` of such non-existent
identifiers. This isn't a problem usually, but due to a upstream bug
in the TypeScript compiler, a runtime error is thrown.

This is because TypeScript is unable to compute a relative path from the
originating source file to the imported source file which _should_
provide the non-existent identifier. An issue for this has been reported
upstream: microsoft/TypeScript#37731. The
issue only surfaces since our migrations don't provide an absolute base
path that is used for resolving the root directories.

To fix this, we ensure that we never use relative paths when parsing
tsconfig files. More details can be found in the TS issue.

Fixes angular#36346.
devversion added a commit to devversion/angular that referenced this issue Apr 3, 2020
@devversion
fix(core): avoid migration error when non-existent symbol is imported
d7cfc9e 
In rare cases a project with configured `rootDirs` that has imports to
non-existent identifiers could fail in the migration.

This happens because based on the application code, the migration could
end up trying to resolve the `ts.Symbol` of such non-existent
identifiers. This isn't a problem usually, but due to a upstream bug
in the TypeScript compiler, a runtime error is thrown.

This is because TypeScript is unable to compute a relative path from the
originating source file to the imported source file which _should_
provide the non-existent identifier. An issue for this has been reported
upstream: microsoft/TypeScript#37731. The
issue only surfaces since our migrations don't provide an absolute base
path that is used for resolving the root directories.

To fix this, we ensure that we never use relative paths when parsing
tsconfig files. More details can be found in the TS issue.

Fixes angular#36346.
kara pushed a commit to angular/angular that referenced this issue Apr 6, 2020
@devversion @kara
fix(core): avoid migration error when non-existent symbol is imported (…
dff52ec 
…#36367)

In rare cases a project with configured `rootDirs` that has imports to
non-existent identifiers could fail in the migration.

This happens because based on the application code, the migration could
end up trying to resolve the `ts.Symbol` of such non-existent
identifiers. This isn't a problem usually, but due to a upstream bug
in the TypeScript compiler, a runtime error is thrown.

This is because TypeScript is unable to compute a relative path from the
originating source file to the imported source file which _should_
provide the non-existent identifier. An issue for this has been reported
upstream: microsoft/TypeScript#37731. The
issue only surfaces since our migrations don't provide an absolute base
path that is used for resolving the root directories.

To fix this, we ensure that we never use relative paths when parsing
tsconfig files. More details can be found in the TS issue.

Fixes #36346.

PR Close #36367
kara pushed a commit to angular/angular that referenced this issue Apr 6, 2020
@devversion @kara
fix(core): avoid migration error when non-existent symbol is imported (…
d43c306 
…#36367)

In rare cases a project with configured `rootDirs` that has imports to
non-existent identifiers could fail in the migration.

This happens because based on the application code, the migration could
end up trying to resolve the `ts.Symbol` of such non-existent
identifiers. This isn't a problem usually, but due to a upstream bug
in the TypeScript compiler, a runtime error is thrown.

This is because TypeScript is unable to compute a relative path from the
originating source file to the imported source file which _should_
provide the non-existent identifier. An issue for this has been reported
upstream: microsoft/TypeScript#37731. The
issue only surfaces since our migrations don't provide an absolute base
path that is used for resolving the root directories.

To fix this, we ensure that we never use relative paths when parsing
tsconfig files. More details can be found in the TS issue.

Fixes #36346.

PR Close #36367
@plantain-00 plantain-00 mentioned this issue May 18, 2020
Open
@sheetalkamat sheetalkamat mentioned this issue Jul 8, 2020
Closed
@sheetalkamat
Copy link
Member

This is working as intended and providing right basePath and handing of paths is expected to be host/api user's responsibility.
The basePath passed in here should have been absolute path instead.

@sheetalkamat sheetalkamat added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript labels Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sheetalkamat sheetalkamat

Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Milestone
TypeScript 4.0.1
Development

No branches or pull requests
3 participants
@devversion @RyanCavanaugh @sheetalkamat