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

Add support for IgnoringWatchFileSystem-instance in watcher #444

Merged
merged 2 commits into from
Jan 20, 2017
Merged

Add support for IgnoringWatchFileSystem-instance in watcher #444

merged 2 commits into from
Jan 20, 2017

Conversation

herschel666
Copy link
Contributor

This fixes the bug described in issue /issues/441.

@johnnyreilly
Copy link
Member

Thanks for contributing! This looks like a reasonable change. Please could you write a test for this to ensure no regressions in future? Probably a comparison test makes most sense. Also, has this been tested with webpack 2? It's due to ship v shortly and so this will need to work with webpack 2 (it probably will)

@johnnyreilly
Copy link
Member

BTW you only need to generate TypeScript 2.1 output. As part of the move to webpack 2 we're only going to run comparison tests against the latest released version of TypeScript (2.1 at present)

@johnnyreilly
Copy link
Member

Now webpack 2 has shipped our test pack targets webpack 2 alone. Just merged the test pack changes

@johnnyreilly
Copy link
Member

@herschel666 - you're probably tearing your hair out right now. Bloody Windows, right? The problem appears to be this

It's expecting this:

chunk    {0} bundle.js (main) 43 bytes [entry] [rendered]
[0] ./.test/issue441/app.ts 43 bytes {0} [built]

But finding this:

chunk    {0} bundle.js (main) 46 bytes [entry] [rendered]
[0] ./.test/issue441/app.ts 46 bytes {0} [built]

Ah - the matter of 3 bytes... such heartache! This is the bane of of our comparison tests; we get these marginal differences which are actually of no matter at all; it fails the test. What I'm planning to do is do a little regex magic on our test pack so we don't actually care about these marginal file size differences in output.txt.

If you want to leave things as they are for now I'll take a look at that (hopefully over the weekend some time but no guarantees) and then we can trigger a re-run and merge. I think you're probably done; I apologise for the pain!

@johnnyreilly
Copy link
Member

Merged - thanks for the contribution!

This will probably go out with our next release which is likely to be our official webpack 2 release. I've put a placeholder in the changelog accordingly: https://github.com/TypeStrong/ts-loader/blob/master/CHANGELOG.md#v200---not-released-yet

@herschel666
Copy link
Contributor Author

Awesome!

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.

2 participants