Tweak globs depending on compile configuration #39
Conversation
novemberborn
changed the title
There was a problem hiding this comment.
Thanks for the PR @alebianco. Sorry for the late response I've been swamped.
I've left some inline comments. Also do you think we need to add tests to cover both use cases?
| @@ -142,7 +142,7 @@ export default function typescriptProvider({negotiateProtocol}) { | |||
| ], | |||
| ignoredByWatcherPatterns: [ | |||
| ...ignoredByWatcherPatterns, | |||
| ...Object.values(relativeRewritePaths).map(to => `${to}**/*.js.map`), | |||
There was a problem hiding this comment.
We'd want to keep this, it's useful if the pre-compilation step outputs source maps. And it should be harmless in case we run tsc ourselves.
index.js
Outdated
| @@ -115,7 +115,7 @@ export default function typescriptProvider({negotiateProtocol}) { | |||
| return false; | |||
| } | |||
|
|
|||
| return rewritePaths.some(([from]) => filePath.startsWith(from)); | |||
| return rewritePaths.some(([to, from]) => filePath.startsWith(compile === 'tsc' ? from : to)); | |||
There was a problem hiding this comment.
Would it make sense to check compile once? Also you mixed up the "from" and "to":
| return rewritePaths.some(([to, from]) => filePath.startsWith(compile === 'tsc' ? from : to)); | |
| return compile === 'tsc' ? rewritePaths.some(([from, to]) => filePath.startsWith(to)) : rewritePaths.some(([from]) => filePath.startsWith(from)); |
index.js
Outdated
| @@ -142,7 +142,7 @@ export default function typescriptProvider({negotiateProtocol}) { | |||
| ], | |||
| ignoredByWatcherPatterns: [ | |||
| ...ignoredByWatcherPatterns, | |||
| ...Object.values(relativeRewritePaths).map(to => `${to}**/*.js.map`), | |||
| ...Object.entries(relativeRewritePaths).map(([to, from]) => `${compile === 'tsc' ? from : to}**`), | |||
There was a problem hiding this comment.
Here too I think the entries are in the shape of [from, to]. And rather than ignoring all files in the directory we should only ignore those with the right extensions. Not sure right now if that should come from any configuration.
| ...Object.entries(relativeRewritePaths).map(([to, from]) => `${compile === 'tsc' ? from : to}**`), | |
| ...Object.entries(relativeRewritePaths).map(([from, to]) => `${compile === 'tsc' ? to : from}**/*.{ts,tsx}`), |
There was a problem hiding this comment.
not sure about ignoring only some file extension, for two reasons:
- a change to a json file (as an example) might trigger tests before the compiler had the chance to run
- depending whether the to or from folder is ignored, the extensions list would change from ts-related to js-related
for the watcher I'd rather ignore the whole folders in order to:
- fully ignore the sources and only react when the compiler changed the output (auto compilation case)
- fully ignore what the compiler might change and only watch the sources (external compilation case)
is there some other scenario I'm not thinking of?
There was a problem hiding this comment.
That makes sense, but it changes the behavior compared to plain AVA. Changing a fixture file should cause tests to re-run.
But to get around that I think we'd need a way to block until compilation is complete?
There was a problem hiding this comment.
oh, didn't think about the fixtures, I haven't used them much until now. It makes sense not to change the standard behaviour.
I'll make the change to ignore only ts,tsx files (or something along those lines) for now 👍
Generates different ignore patterns depending on the pre-compilation configuration:
When the pre-compilation is on:
When the pre-compilation is off:
Fixes #33.