Allow an import of "foo.js" to be matched by a file "foo.ts" #8895
Conversation
|
Hi @andy-ms, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
TTYL, MSBOT; |
| if (ext === ".tsx" && state.skipTsx) { | ||
| return undefined; | ||
| if (state.skipTsx) | ||
| extensions = filter(extensions, ext => ext !== "tsx"); |
There was a problem hiding this comment.
Shouldn't this still have a dot in it?
There was a problem hiding this comment.
Yes it should! I'll add tests so this error is detected.
There was a problem hiding this comment.
Also, noticed that I forgot the curly braces on this. Maybe we should add a lint rule?
There was a problem hiding this comment.
i would rather we checked this tryLoad to avoid the array allocation for every module lookup.
ghost
| } | ||
| return tryLoad(fileExtensionIs(candidate, ext) ? candidate : candidate + ext); | ||
| }); | ||
| if (keepOrAddExtension) { |
| return fileExtensionIs(path, extension) ? path.substring(0, path.length - extension.length) : undefined; | ||
| } | ||
|
|
||
| export function getFileExtension(path: string): string { |
There was a problem hiding this comment.
Consider making the return type annotation string | undefined
| } | ||
|
|
||
| export function getFileExtension(path: string): string { | ||
| const dot = path.lastIndexOf("."); |
There was a problem hiding this comment.
What if the dot is somewhere else in the path, e.g. /foo/some.dir/filename ?
There was a problem hiding this comment.
Turns out we don't need this function, so removing it.
|
That's a bigger change than I would have expected. What doesn't work if you just remove a |
|
If a file is named "foo.js.ts", currently imports of "foo.js" will work. If we eagerly strip the ".js" extension from the import it will stop working. This might be an acceptable breaking change, though. |
ghost
mentioned this pull request
|
👍 |
ghost
mentioned this pull request
ghost
mentioned this pull request
ghost
mentioned this pull request
Fixes #4595