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

Use bundled typescript #138

Merged
merged 1 commit into from Dec 9, 2018
Merged

Use bundled typescript #138

merged 1 commit into from Dec 9, 2018

Conversation

guybedford
Copy link
Contributor

The issue fixed here is that the typescript loader loads typescript using a dynamic require in https://github.com/TypeStrong/ts-loader/blob/master/src/compilerSetup.ts#L15.

This is fixed using the previous dynamic require technique of allowing webpack to support a numeric require where compiler is simply the numeric require.resolve('typescript') replacement provided by Webpack.

We do this by patching the empty context module to attempt to load that numeric id when it can.

With this fixed, the remaining bug was that TypeScript loads its own types from the lib directory, but with logic that isn't statically analyzable by ncc, something like the following:

processRootFile(ts.combinePaths(defaultLibraryPath, libFileName), /*isDefaultLib*/ true, /*ignoreNoDefaultLib*/ true);

where the asset emission would work if the above was path.join(__dirname, 'lib', libFileName) but doesn't work here because we can't statically analyze either defaultLibraryPath, libFileName or ts.combinePaths.

The work-around here was just like the webpack builtins - to manually relocate the lib folder into the ncc dist folder, which surprisingly then all seems to work out ok.

It would be worth testing it further though with various configurations to be sure...

package.json Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Dec 9, 2018

Codecov Report

Merging #138 into master will decrease coverage by 1.07%.
The diff coverage is 11.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #138      +/-   ##
==========================================
- Coverage   74.94%   73.86%   -1.08%     
==========================================
  Files          10       10              
  Lines         455      463       +8     
==========================================
+ Hits          341      342       +1     
- Misses        114      121       +7
Impacted Files Coverage Δ
src/loaders/ts-loader.js 0% <0%> (ø) ⬆️
src/index.js 93.15% <100%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c19e81...3e10fc7. Read the comment docs.

@rauchg rauchg merged commit d349b38 into master Dec 9, 2018
@rauchg rauchg deleted the ts-bundle branch December 9, 2018 23:27
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.

None yet

4 participants