Assist: ts-loader#945 #1
Conversation
) Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 3.12.1 to 3.13.1. - [Release notes](https://github.com/nodeca/js-yaml/releases) - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](nodeca/js-yaml@3.12.1...3.13.1) Signed-off-by: dependabot[bot] <[email protected]>
Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 3.12.0 to 3.13.1. - [Release notes](https://github.com/nodeca/js-yaml/releases) - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](nodeca/js-yaml@3.12.0...3.13.1) Signed-off-by: dependabot[bot] <[email protected]>
Bumps [clean-css](https://github.com/jakubpawlowicz/clean-css) from 4.1.9 to 4.1.11. - [Release notes](https://github.com/jakubpawlowicz/clean-css/releases) - [Changelog](https://github.com/jakubpawlowicz/clean-css/blob/master/History.md) - [Commits](clean-css/clean-css@v4.1.9...v4.1.11) Signed-off-by: dependabot[bot] <[email protected]>
- track which files have been renamed by appendSuffixTo and exclude them from needing to be in rootFileNames - check for project references before rootFileNames - fix up remaining test tsconfigs
…tensions the same way ts-loader does
andrewbranch
left a comment
andrewbranch
left a comment
There was a problem hiding this comment.
@davazp thanks so much for the awesome work so far! Let me know what you think about this.
| return configParseResult; | ||
| } | ||
|
|
||
| function getSuffixAppendingParseConfigHost( |
There was a problem hiding this comment.
This is the heart of my changes. It solves the appendSuffix problem by providing TypeScript an interface to the file system that actually acts on the file names that ts-loader tracks, i.e. renamed with the .ts or .tsx suffix appropriately. The result is that configParseResult.fileNames will include things like foo.vue.tsx when ts-loader is told to assume foo.vue is a TSX file.
So I think this is actually a case where we can reliably be certain that the files that exist will not be in the
tsconfig.jsonand in fact cannot be.
Au contraire mon frère, @johnnyreilly 😄
There was a problem hiding this comment.
This is the heart of my changes. It solves the appendSuffix problem by providing TypeScript an interface to the file system that actually acts on the file names that ts-loader tracks
That's clever! But could this be a bit unintuitive for the users? Some patterns would match actual files but not transformed files. For instance, patterns like src/**/*.vue, or just main.vue inside "files". Right?
Of course the user can specify patterns on the transformed files. But then the appendSuffix is not that transparent anymore. Maybe that's not wrong, but still pretty confusing. 🤔
| ); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Moved this to come after project reference handling
| loaderContext: webpack.loader.LoaderContext, | ||
| instance: TSInstance | ||
| instance: TSInstance, | ||
| options: LoaderOptions |
There was a problem hiding this comment.
instance.loaderOptions !== options, ouch 😖 Pretty sure we do this wrong in lots more places, but this one was needed to get tests passing
| if (!instance.rootFileNames.has(filePath)) { | ||
| instance.version!++; | ||
| if (options.onlyCompileBundledFiles) { | ||
| instance.rootFileNames.add(filePath); |
There was a problem hiding this comment.
Need to add the file manually, otherwise rootFileNames will just be declarations
|
|
||
| getScriptFileNames: () => | ||
| [...files.keys()].filter(filePath => filePath.match(scriptRegex)), | ||
| getScriptFileNames: () => Array.from(instance.rootFileNames), |
There was a problem hiding this comment.
This was the change I mentioned before that fixes TypeStrong#949. Doing this exposed a couple other problems, like needing to add the file to instance.rootFileNames when onlyCompileBundledFiles is true.
2 participants
This should, I think, take care of the remaining test failures in TypeStrong#945. I'll leave a review to explain the changes inline.