fix: support TS resolution in JS files when allowJs is set
#535
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
I didn't fully understand how the tests are structured/split up; I'd be more than happy to adjust things here in response to feedback.
There was a problem hiding this comment.
For context, the idea behind the smoke tests here is to test with minimal number of child processes; the other tests are slow primarily because it spawns so many children.
| @@ -218,6 +218,14 @@ const files = { | |||
|
|
|||
| 'file.txt': 'hello', | |||
|
|
|||
| 'import-typescript-parent.js': sourcemap.tag` | |||
| import './import-typescript-child.js'; | |||
There was a problem hiding this comment.
Can you move this inside import-from-js.js?
There was a problem hiding this comment.
I can, though that will require adding --tsconfig tsconfig/tsconfig-allowJs.json to that test's arguments, at which point I think there wouldn't be any tests that cover the case where TS resolution rules aren't followed. If that's good with you, I'll do the move and delete the allowJs in tsconfig.json test.
There was a problem hiding this comment.
Ah good point, let's keep it this way then. Thanks for catching it!
| `, | ||
|
|
||
| 'import-typescript-child.ts': sourcemap.tag` | ||
| console.log('imported'); |
There was a problem hiding this comment.
Make this allowJs or something more unique that can be matched from within the 'from .js' test
|
Really appreciate this PR, thanks @nwalters512 ! BTW I had a vision to leverage get-tsconfig like this https://github.com/esbuild-kit/cjs-loader/pull/34/files but I think there are other problems with that approach and this is a good solution for now. 🙌 |
|
And thank you for maintaining this excellent package! It's been a breath of fresh air after |
privatenumber
changed the title
allowJs is set
|
🎉 This issue has been resolved in v4.7.3 If you appreciate this project, please consider supporting this project by sponsoring ❤️ 🙏 |
Closes #135.
I tested this locally and it works as expected in my project. I also added a new test which fails on
developbut passes after these changes.