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

[Compiler Options] Account for allowJs compiler option #316

Closed
dschnare opened this issue Oct 14, 2016 · 10 comments
Closed

[Compiler Options] Account for allowJs compiler option #316

dschnare opened this issue Oct 14, 2016 · 10 comments

Comments

@dschnare
Copy link

Currently, even though the allowJs compiler option has been defined in a tsconfig.json or via the ts.compilerOptions config, JavaScript modules will not be found. I have resolved this issue, at least for my needs, by changing line 95 in index.ts.

Original: const scriptRegex = /\.tsx?$/i;
Changed: const scriptRegex = /\.[jt]sx?$/i;

I haven't ran into any issues yet. If anyone has time to fully test this use case out. I would greatly appreciate your efforts. I currently don't have time to dig into this for a few weeks. There may other cases in the source code where the file extension match needs to take into account the allowJs option as well.

@johnnyreilly
Copy link
Member

Thanks for raising this issue @dschnare - if it's as simple as that (and it may well be) then I'd like to include this in a future release. If you get the chance to dig into this we'd greatly appreciate a PR.

@johnnyreilly
Copy link
Member

johnnyreilly commented Oct 16, 2016

Actually I may look at this myself- fixing this as it may well resolve #278

@johnnyreilly
Copy link
Member

johnnyreilly commented Oct 16, 2016

Hi @dschnare,

I'm planning to release a version of ts-loader that includes your suggested fix. Would you be able to supply me with a repo that demostrates the problem when this is not available?

Try as I might I've struggled to reproduce the problem myself. js modules seem to be resolved no matter what.... which is odd.

Either way, I'd like to ensure that I have a test to cover this in terms of preventing regressions. If you could help that'd be great.

@dschnare
Copy link
Author

Hey John,

It seems to occur when the entry point module is a JS file. It also
occurred when a TS entry point imports a JS module. I'll put up a repo for
you though in the next 24 hours.

On Oct 16, 2016 3:08 PM, "John Reilly" notifications@github.com wrote:

Hi @dschnare https://github.com/dschnare,

I'm planning to release a version of ts-loader that includes your
suggested fix. Would you be able to supply me with a repo that demostrates
the problem when this is not available? Try as I might I've struggled to
reproduce the problem myself. js seems to be resolved no matter what and
I'd like to ensure that I have a test to cover this in terms of preventing
regressions.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#316 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA2hta1aTp-QwUrL3kqpkjqRqJeUyOg2ks5q0nYYgaJpZM4KXYVy
.

@johnnyreilly
Copy link
Member

johnnyreilly commented Oct 16, 2016

It seems to occur when the entry point module is a JS file. It also occurred when a TS entry point imports a JS module.

Funny - I couldn't get that not to work! I will look forward to the repo and look to add / amend tests accordingly 👍

Thanks chap! Just readying the 0.9.3 release now which adds allowJs support - should ship in an hour or so.

@dschnare
Copy link
Author

@johnnyreilly version 0.9.3 resolved the issue I was having. I have pushed up a repo that still uses 0.9.2 and can demonstrate the issue.

Steps to reproduce:

  1. run npm install
  2. run npm run build:web

The build:web npm-script uses Webpack to build out a browser-ready version of this module.

@johnnyreilly
Copy link
Member

Excellent - thanks!

@johnnyreilly
Copy link
Member

Great - I've used your repo to make a some better allowJs tests: #325

@dschnare
Copy link
Author

This issue has been re-introduced in version 1.3.3.

@johnnyreilly
Copy link
Member

See discussion in #388 @dschnare - this was intentional due to the implications of introducing that behaviour as a default (caused problems in certain scenarios). The behaviour you are after is still available but behind the entryFileIsJs ts-loader option. See the readme for details

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

No branches or pull requests

2 participants