Skip to content
This repository has been archived by the owner on Jan 26, 2019. It is now read-only.

Syntax-errors in tests fails to compile, but are not picked up by watch #152

Open
GeeWee opened this issue Aug 30, 2017 · 16 comments
Open

Comments

@GeeWee
Copy link
Contributor

GeeWee commented Aug 30, 2017

Is this a bug report?

Bug report

Can you also reproduce the problem with npm 4.x?

I'm running npm 5.3
Same bug on yarn 0.27.5

Environment

  1. npm ls react-scripts-ts (if you haven’t ejected): 2.6.0
  2. node -v: 8.4 currently, issue is also on 7.x branch
  3. npm -v: 5.3
  4. yarn --version (if you use Yarn): 0.27.5

Then, specify:

  1. Operating system: Windows 10 for me. Problem also occurs on OSX Sierra.

Bug report

If you run yarn/npm start with a syntax error in your tests, it crashes and reports a tsc error. However if you then fix the syntax error, an incremental recompilation is not triggered.

I'd expect it to either not error on the tests at all, or also pick up the changes when the tests are changed.

I think perhaps this might have something to do with webpack building a dependency graph, and only compiling files that are referenced by the entry point.

Support for this thesis: The test files are not picked up for recompilation. However if i export something from the test file, and then import it into e.g. app.tsx, the test files are picked up for incremental recompilation, and it's possible to fix the syntax error and continue developing.

@wmonk
Copy link
Owner

wmonk commented Aug 30, 2017

Is this a duplicate of #148

@GeeWee
Copy link
Contributor Author

GeeWee commented Aug 30, 2017

It doesn't look like it. This is replicatable with both npm and yarn, and only exists in files that are not referenced by the entry point of the app.

@DorianGrey
Copy link
Collaborator

Yes, webpack should not care about the test (i.e. .test.tsx) files since they are not referenced from the entry point. It's a bit surprising why this happens on the first run.
For the moment, this could be fixed by adding

"src/**/*.test.tsx"

to the list of exclusion in the tsconfig.json.

It seems to have something to do with the way the files are named - if App.test.tsx is renamed to App2.test.tsx or App.spec.tsx, everything works fine.

@GeeWee
Copy link
Contributor Author

GeeWee commented Aug 30, 2017

I think excluding tests might mess with the actual testing though 😛
The file naming is really strange though, and I'm not quite sure I follow. Can you elaborate on that?

@DorianGrey
Copy link
Collaborator

I think excluding tests might mess with the actual testing though

I does not, actually. The exclude is not taken into consideration when processing the files via ts-jest.

The file naming is really strange though, and I'm not quite sure I follow. Can you elaborate on that?

I've just created a fresh project as noted in the docs, which also creates a default test App.test.tsx in the src directory. Modified this file to something that should cause a typescript compiler error to reproduce your test case - after running yarn start, I got the same result as you've faced.
Afterwards, I've renamed this file to App.spec.tsx (which is also covered by the matching for jest), and after running yarn start again, no error occurred, although it still included the change that raised the error before. It raised correctly when executing the tests.

Tested this renaming since it seems that webpack picks up both App.tsx and App.test.tsx - I don't see any other way how the test file might get involved here. However, I'm not sure why it does; it is not an entry point, nor referenced anywhere. Same should happen for other test files named .test.tsx, actually.

@GeeWee
Copy link
Contributor Author

GeeWee commented Aug 30, 2017

Ah, that might be true. Still, I've had excludes bite my ass, when e.g. using storybook, so I'd prefer to not go that route.

Huh, that seems really strange, so it's the *.test.tsx? pattern it picks up on at the start.

I looked at the source of this, and it seems to simply fire up webpack with ts-loader, so I'm not quite sure what's going on. Perhaps there are some diagnostic switches we could toggle? tsc has some, but I'm not sure how we'd go about turning them on here.

@DorianGrey
Copy link
Collaborator

DorianGrey commented Aug 30, 2017

Regularly, it would be possible to configure webpack's stats output to provide details about which modules got included and why, but that won't work here due to the way it's configured for CRA.

I've just faced an even more curious behavior: I wanted to switch back the test project to raise the error again, so I've renamed App.spec.tsx to App.test.tsx again ... and no error occured when running yarn start. It seems that the problem with accidentially included *.test.tsx files only occurs until they get renamed once ... ?
That does not make any sense. Really 😳

@GeeWee
Copy link
Contributor Author

GeeWee commented Aug 30, 2017

That's odd. I'd wager that's because of a caching issue or the earlier behaviour was a fluke of some sort - what'ya reckon?

@DorianGrey
Copy link
Collaborator

DorianGrey commented Aug 30, 2017

I'm not sure what happens, actually. Recreated a fresh version of the project - everything worked as mentioned above. Did that a second time - now neither renaming nor any other of the changes mentioned above works, except adding the entry to the exclude definition.

This is definitely odd ... weird... and a bit scary.

€dit: OK, after testing various ways to exclude the test file manually (e.g. modifying webpack config) the only change that reliably excludes the test files from being picked up was adding the "src/**/*.test.tsx" glob to the exclude entry in the tsconfig.
It seems that this is standard typescript behavior - I've found some reports about that behavior in both typescript loaders available, including ts-loader used by this project, and all of them point into this direction:
s-panferov/awesome-typescript-loader#359
TypeStrong/ts-loader#544

@wmonk
Copy link
Owner

wmonk commented Sep 7, 2017

Can I get a quick tldr of where this issue is at? @DorianGrey reading your last comment it seems that this is a TS issue?

@GeeWee
Copy link
Contributor Author

GeeWee commented Sep 7, 2017

Seems like it's an at-loader issue: s-panferov/awesome-typescript-loader#364

@DorianGrey
Copy link
Collaborator

Seems like it's an at-loader issue: s-panferov/awesome-typescript-loader#364

That would be curious, since it's not used in this project ;)

Can I get a quick tldr of where this issue is at?

@wmonk : From what I've read in the issues linked in my post above (and what was linked from those), it is a TS feature. TS assumes to be responsible for every file in the current project as long as there is no explicit include list to restrict this list or an exclude to tell which files should be ignored. As a result, it will check them, even if they are not transpiled or referenced. However, recompilation only happens if one of the files that webpack watches changes - which does not hold true for the test files, since they are not referenced by the application code.
This behavior can only be avoided by adding the "src/**/*.test.tsx" glob to the exclude entry in the tsconfig.json. Adding this option won't have an impact on test execution, since ts-jest uses transpileModule internally, which only takes care of the compiler options, not of any other config entry.

@ianschmitz
Copy link
Contributor

Related to @DorianGrey comment: #159

@gsteacy
Copy link

gsteacy commented Sep 23, 2017

This behavior can only be avoided by adding the "src/**/*.test.tsx" glob to the exclude entry in the tsconfig.json.

This will throw off editor type checking though. For editors which strictly respect tsconfig.json it will stop type checking altogether. For others, eg. VS Code, it will just create edge cases where the type checking behaves unexpectedly because some files are part of the TS project and others are not. For example, you'll type checking errors if you reference ambient types declared in an included file from an excluded file. Furthermore, the excluded files are checked with the default compiler options. The editor won't complain about unused locals in the tests because noUnusedLocals is not enabled by default.

If you're happy to eject and keep your tests separate from your source (eg. a separate test directory under src) then a better option might be to have a tsconfig.json in the root, in the src directory, and in the test directory. Most compiler options can be configured in the root one, but it should not include or exclude any files explicitly. The one in src can extend the root one and include all relevant files under src and exclude the tests. The one in test can override the exclusion of the tests. You'd point webpack at the tsconfig.json in src. ts-jest can continue to use tsconfig.test.json since it only cares about compiler options anyway.

Unfortunately I don't think this can be achieved without ejecting because you can't configure the tsconfig.json used by webpack. I don't know how it could be achieved if you don't separate your tests either. "Broken" editor type checking for tests might be a necessary trade off. That's a pain because ts-jest doesn't do it either.

@mikew
Copy link

mikew commented Dec 19, 2017

Unfortunately I don't think this can be achieved without ejecting because you can't configure the tsconfig.json used by webpack.

ts-node supports the TS_NODE_PROJECT environment variable. Could something like that work here?

I'm in an odd position where TS errors in tests appear in the frontend, and aren't caught when running the test script.

@gilbsgilbs
Copy link

gilbsgilbs commented May 26, 2018

This issue is really frustrating on a daily basis. It breaks my workflow (don't care about tests typings or linting while I'm working on a source file) and it's even more annoying to have to restart the dev server or save a random source file to force the refresh.

Excluding tests files from tsconfig isn't the perfect workaround because tslint will then also ignore them. You have to lint them manually (or in CI to enforce it a bit more). Moreover, loosing consistency of TS support in IDEs such as VSCode as highlited by @gsteacy is indeed a significant downside.

Maybe one inelegant solution could be to use a different tsconfig for the dev server. The default tsconfig.json wouldn't change, but the tsconfig.devServer.json would exclude test files. This way, typings and linting would keep working as expected everywhere, except that the dev server wouldn't care about test files anymore. Yet another tsconfig file though 👎 . And the original tsconfig exclusion list would certainly need to be copy-pasted from the original tsconfig which is clunky to say the least.

Any other suggestion to fix this is welcomed. I don't see this changing with 2.0/webpack 4, is it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants