Skip to content

Fix bug with webpack resolution #102

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

Merged
merged 2 commits into from
Nov 10, 2015
Merged

Fix bug with webpack resolution #102

merged 2 commits into from
Nov 10, 2015

Conversation

jbrantly
Copy link
Member

@jbrantly jbrantly commented Nov 8, 2015

This addresses #92. webpack resolution was not working correctly.

From that issue:

So I've looked into this a bit and there are multiple cascading issues. The first issue is that there is a bug in ts-loader where the webpack lookup was failing (ts-loader first tries to resolve a file using webpack's resolution strategy). Since that was failing, the loader fell back on TypeScript's module resolution strategy. In this case TypeScript was unable to find the .tsx file because the jsx option was not specified in the compiler options, so that failed as well resulting in the "cannot find module" message. However, if you switched moduleResolution to node then TypeScript magically decided to include .tsx files in its lookup. Kind of weird if you ask me and possibly a bug that .tsx extension handling is done differently between the two strategies. In any case, by fixing the original bug the TypeScript resolution "bug" doesn't come into play.

The test here mimics the original issue of trying to import a tsx without specifying jsx or moduleResolution.

@jbrantly
Copy link
Member Author

jbrantly commented Nov 8, 2015

I was able to reproduce an issue reported in gitter and confirm that it's also resolved by this PR. Adding that scenario as an additional test. @Coderah

@Coderah
Copy link

Coderah commented Nov 8, 2015

@jbrantly Great! Thank you for remembering and informing me of this, really appreciate that.

@jbrantly
Copy link
Member Author

@blakeembrey When you get a moment could you review this? (forgot to cc originally). Thanks!

@blakeembrey
Copy link
Member

LGTM 👍

jbrantly added a commit that referenced this pull request Nov 10, 2015
Fix bug with webpack resolution
@jbrantly jbrantly merged commit 59bd79b into master Nov 10, 2015
@johnnyreilly johnnyreilly deleted the issue92 branch July 13, 2017 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants