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

feat: adjust TS2691 message for .ts import sources #42184

Merged
merged 3 commits into from
Jan 5, 2021

Conversation

ctjlewis
Copy link
Contributor

@ctjlewis ctjlewis commented Jan 3, 2021

The advice given by error TS2691 is confusing, and following its instructions produces output which throws ERR_MODULE_NOT_FOUND at runtime when outputting an ES module, i.e. when moduleKind >= ModuleKind.ES2015.

An import for a module located at ./test.ts must be specified as import ... from './test.js' in order to get a valid ES module out, but users are directed to rename it to ./test. This error message should be updated so users are given the correct advice based on their use case, and are not confused why their output throws despite the source being perfectly valid TS. (The ./test format can be used to generate valid CommonJS modules, just not ES modules.)

Closes #42151.

Current behavior

import { test } from './test.ts'

Currently throws TS2691 which says:

An import path cannot end with a '.ts' extension. Consider importing './test' instead.ts(2691)

This adjustment does silence the compiler, but the compiled JS looks like:

import { test } from './test';
console.log(test);

Which is not a valid ES module, as the import source is located at ./test.js. Executing the output throws ERR_MODULE_NOT_FOUND.

Suggested changes

This PR adjusts checker.ts to detect if the output is an ES module, and if so advises the user to replace ./import.ts with ./import.js instead of ./import.

After these changes, trying to import ./test.js while outputting an ES module throws:

An import path cannot end with a '.ts' extension. Consider importing './test.js' instead.ts(2691)

Which will compile to a valid ES module that behaves as expected.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jan 3, 2021
Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

Can you add a test that shows this error? Thanks!

src/compiler/checker.ts Show resolved Hide resolved
@ctjlewis
Copy link
Contributor Author

ctjlewis commented Jan 4, 2021

Sure @andrewbranch - if you see this in time, is there a way to run just a single test in this workspace, rather than running the whole suite with gulp test?

@andrewbranch
Copy link
Member

npx gulp runtests --tests substringOfTestFileName

@andrewbranch
Copy link
Member

super sorry for pinging you

Don’t be! Contributions are always appreciated and we expect first-time contributors to have some questions 👍

@ghost
Copy link

ghost commented Jan 5, 2021

CLA assistant check
All CLA requirements met.

Comment on lines +1 to +3
tests/cases/compiler/user.ts(1,15): error TS2691: An import path cannot end with a '.ts' extension. Consider importing './x' instead.
tests/cases/compiler/user.ts(2,15): error TS2691: An import path cannot end with a '.tsx' extension. Consider importing './y' instead.
tests/cases/compiler/user.ts(3,15): error TS2691: An import path cannot end with a '.d.ts' extension. Consider importing './z' instead.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change for CJS module target.

Comment on lines +1 to +3
tests/cases/compiler/user.ts(1,15): error TS2691: An import path cannot end with a '.ts' extension. Consider importing './x.js' instead.
tests/cases/compiler/user.ts(2,15): error TS2691: An import path cannot end with a '.tsx' extension. Consider importing './y.js' instead.
tests/cases/compiler/user.ts(3,15): error TS2691: An import path cannot end with a '.d.ts' extension. Consider importing './z.js' instead.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change for ES module target.

Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

Thanks @ctjlewis!

@andrewbranch andrewbranch merged commit 7a5aadc into microsoft:master Jan 5, 2021
Zzzen pushed a commit to Zzzen/TypeScript that referenced this pull request Jan 16, 2021
* Adjust TS2691 message for .ts import sources

* Only ModuleKind is needed for TS2691 logic

* Added tests for TS2691
@ctjlewis ctjlewis changed the title Adjust TS2691 message for .ts import sources feat: adjust TS2691 message for .ts import sources Jul 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeScript cannot emit valid ES modules due to file extension issue
4 participants