-
-
Notifications
You must be signed in to change notification settings - Fork 433
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 allowTsInNodeModules option for importing .ts files from node_modules. #773
Merged
johnnyreilly
merged 10 commits into
TypeStrong:master
from
aelawson:aelawson/allow-node-modules
May 7, 2018
Merged
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
f994a25
Add allowTsInNodeModules loader option.
6c9cdb9
Add error text for allowTsInNodeModules option when importing .ts files
bb13166
Update nodeModulesMeaningfulErrorWhenImportingTs test outputs.
31bec1f
Add test for enabling allowTsInNodeModules option.
84d7aa7
Add option description to README.md
2c6e47c
Change allowTsInNodeModules from comparison test to execution test.
f4ceb49
Fix indentation.
e0a0ff7
Linkify GitHub issue reference.
517af98
Update error message logic to use ternary.
c3af054
Slight change in output for nodeModulesMeaningfulErrorWhenImportingTs
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,10 +69,17 @@ function successLoader( | |
: getEmit(rawFilePath, filePath, instance, loader); | ||
|
||
if (outputText === null || outputText === undefined) { | ||
const additionalGuidance = | ||
filePath.indexOf('node_modules') !== -1 | ||
? '\nYou should not need to recompile .ts files in node_modules.\nPlease contact the package author to advise them to use --declaration --outDir.\nMore https://github.com/Microsoft/TypeScript/issues/12358' | ||
: ''; | ||
let additionalGuidance: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we turn this into a ternary please? Something like:
|
||
|
||
if (!options.allowTsInNodeModules && filePath.indexOf('node_modules') !== -1) { | ||
additionalGuidance = " By default, ts-loader will not compile .ts files in node_modules.\n" + | ||
"You should not need to recompile .ts files there, but if you really want to, use the allowTsInNodeModules option.\n" + | ||
"See: https://github.com/Microsoft/TypeScript/issues/12358"; | ||
} | ||
else { | ||
additionalGuidance = ""; | ||
} | ||
|
||
throw new Error( | ||
`Typescript emitted no output for ${filePath}.${additionalGuidance}` | ||
); | ||
|
@@ -147,7 +154,8 @@ const validLoaderOptions: ValidLoaderOptions[] = [ | |
'happyPackMode', | ||
'getCustomTransformers', | ||
'reportFiles', | ||
'experimentalWatchApi' | ||
'experimentalWatchApi', | ||
'allowTsInNodeModules' | ||
]; | ||
|
||
/** | ||
|
@@ -199,7 +207,8 @@ function makeLoaderOptions(instanceName: string, loaderOptions: LoaderOptions) { | |
onlyCompileBundledFiles: false, | ||
reportFiles: [], | ||
// When the watch API usage stabilises look to remove this option and make watch usage the default behaviour when available | ||
experimentalWatchApi: false | ||
experimentalWatchApi: false, | ||
allowTsInNodeModules: false | ||
} as Partial<LoaderOptions>, | ||
loaderOptions | ||
); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
15 changes: 7 additions & 8 deletions
15
.../comparison-tests/nodeModulesMeaningfulErrorWhenImportingTs/expectedOutput-2.8/output.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,13 @@ | ||
Asset Size Chunks Chunk Names | ||
bundle.js 3.75 KiB main [emitted] main | ||
bundle.js 3.77 KiB main [emitted] main | ||
Entrypoint main = bundle.js | ||
[./app.ts] 79 bytes {main} [built] | ||
[./node_modules/a/index.ts] 517 bytes {main} [built] [failed] [1 error] | ||
[./node_modules/a/index.ts] 568 bytes {main} [built] [failed] [1 error] | ||
|
||
ERROR in ./node_modules/a/index.ts | ||
Module build failed: Error: Typescript emitted no output for node_modules\a\index.ts. | ||
You should not need to recompile .ts files in node_modules. | ||
Please contact the package author to advise them to use --declaration --outDir. | ||
More https://github.com/Microsoft/TypeScript/issues/12358 | ||
at successLoader (dist\index.js:39:15) | ||
at Object.loader (dist\index.js:21:12) | ||
Module build failed: Error: Typescript emitted no output for node_modules/a/index.ts. By default, ts-loader will not compile .ts files in node_modules. | ||
You should not need to recompile .ts files there, but if you really want to, use the allowTsInNodeModules option. | ||
See: https://github.com/Microsoft/TypeScript/issues/12358 | ||
at successLoader (dist/index.js:45:15) | ||
at Object.loader (dist/index.js:21:12) | ||
@ ./app.ts 3:8-20 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
12 changes: 6 additions & 6 deletions
12
test/comparison-tests/validateLoaderOptionNames/expectedOutput-2.8/output.txt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,14 @@ | ||
Asset Size Chunks Chunk Names | ||
bundle.js 3.51 KiB main [emitted] main | ||
bundle.js 3.53 KiB main [emitted] main | ||
Entrypoint main = bundle.js | ||
[./app.ts] 728 bytes {main} [built] [failed] [1 error] | ||
[./app.ts] 766 bytes {main} [built] [failed] [1 error] | ||
|
||
ERROR in ./app.ts | ||
Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption | ||
|
||
Please take a look at the options you are supplying; the following are valid options: | ||
silent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi | ||
silent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules | ||
|
||
at validateLoaderOptions (dist\index.js:103:19) | ||
at getLoaderOptions (dist\index.js:66:5) | ||
at Object.loader (dist\index.js:15:21) | ||
at validateLoaderOptions (dist/index.js:110:19) | ||
at getLoaderOptions (dist/index.js:72:5) | ||
at Object.loader (dist/index.js:15:21) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
/* eslint-disable no-var, strict */ | ||
'use strict'; | ||
var path = require('path'); | ||
var webpack = require('webpack'); | ||
var webpackConfig = require('./webpack.config.js'); | ||
var reporterOptions = require('../../reporterOptions'); | ||
|
||
module.exports = function(config) { | ||
config.set({ | ||
browsers: [ 'ChromeHeadless' ], | ||
|
||
files: [ | ||
// This loads all the tests | ||
'main.js' | ||
], | ||
|
||
port: 9876, | ||
|
||
frameworks: [ 'jasmine' ], | ||
|
||
logLevel: config.LOG_INFO, //config.LOG_DEBUG | ||
|
||
preprocessors: { | ||
'main.js': [ 'webpack', 'sourcemap' ] | ||
}, | ||
|
||
webpack: { | ||
devtool: 'inline-source-map', | ||
mode: webpackConfig.mode, | ||
module: webpackConfig.module, | ||
resolve: webpackConfig.resolve, | ||
|
||
// for test harness purposes only, you would not need this in a normal project | ||
resolveLoader: webpackConfig.resolveLoader | ||
}, | ||
|
||
webpackMiddleware: { | ||
quiet: true, | ||
stats: { | ||
colors: true | ||
} | ||
}, | ||
|
||
// reporter options | ||
mochaReporter: reporterOptions | ||
}); | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
const testsContext = require.context('./', true, /\.tests\.ts(x?)$/); | ||
testsContext.keys().forEach(testsContext); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
{ | ||
"name": "basic", | ||
"license": "MIT", | ||
"version": "1.0.0", | ||
"main": "index.js", | ||
"dependencies": { | ||
"whitelistedModule": "file:../../testPackages/whitelistedModule", | ||
"whitelistedFiles": "file:../../testPackages/whitelistedFiles" | ||
}, | ||
"devDependencies": { | ||
"@types/jasmine": "^2.5.35", | ||
"jasmine-core": "^2.3.4" | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import whitelistedModule = require('whitelistedModule'); | ||
|
||
export function get() { | ||
return whitelistedModule; | ||
} |
5 changes: 5 additions & 0 deletions
5
test/execution-tests/allowTsInNodeModules/src/whitelisted_file.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import whitelistedFile = require('whitelistedFiles/file'); | ||
|
||
export function get() { | ||
return whitelistedFile; | ||
} |
13 changes: 13 additions & 0 deletions
13
test/execution-tests/allowTsInNodeModules/test/app.tests.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
describe("whitelisted", () => { | ||
it("module can be imported", () => { | ||
const whitelisted = require('../src/whitelisted'); | ||
|
||
expect(whitelisted.get()).toBe("my whitelisted module"); | ||
}); | ||
|
||
it("file can be imported", () => { | ||
const whitelisted = require('../src/whitelisted_file'); | ||
|
||
expect(whitelisted.get()).toBe("a whitelisted file"); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"compilerOptions": { }, | ||
"include": [ | ||
"./node_modules/whitelistedModule" | ||
], | ||
"files": [ | ||
"./node_modules/whitelistedFiles/file.ts" | ||
] | ||
} |
23 changes: 23 additions & 0 deletions
23
test/execution-tests/allowTsInNodeModules/webpack.config.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
var path = require('path') | ||
|
||
module.exports = { | ||
mode: 'development', | ||
entry: [ | ||
'./src/whitelisted.ts', | ||
'./src/whitelisted_file.ts' | ||
], | ||
output: { | ||
filename: 'bundle.js' | ||
}, | ||
resolve: { | ||
extensions: ['.ts', '.js'] | ||
}, | ||
module: { | ||
rules: [ | ||
{ test: /\.ts$/, loader: 'ts-loader', options: { allowTsInNodeModules: true } } | ||
] | ||
} | ||
} | ||
|
||
// for test harness purposes only, you would not need this in a normal project | ||
module.exports.resolveLoader = { alias: { 'ts-loader': path.join(__dirname, "../../../index.js") } } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. | ||
# yarn lockfile v1 | ||
|
||
|
||
"@types/jasmine@^2.5.35": | ||
version "2.8.6" | ||
resolved "https://registry.yarnpkg.com/@types/jasmine/-/jasmine-2.8.6.tgz#14445b6a1613cf4e05dd61c3c3256d0e95c0421e" | ||
|
||
jasmine-core@^2.3.4: | ||
version "2.99.1" | ||
resolved "https://registry.yarnpkg.com/jasmine-core/-/jasmine-core-2.99.1.tgz#e6400df1e6b56e130b61c4bcd093daa7f6e8ca15" | ||
|
||
"whitelistedFiles@file:../../testPackages/whitelistedFiles": | ||
version "1.0.0" | ||
|
||
"whitelistedModule@file:../../testPackages/whitelistedModule": | ||
version "1.0.0" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
var whitelistedFile = "a whitelisted file"; | ||
|
||
export = whitelistedFile; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"name": "whitelistedFiles", | ||
"version": "1.0.0", | ||
"main": "file.ts" | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
var whitelistedModule = "my whitelisted module"; | ||
|
||
export = whitelistedModule; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"name": "whitelistedModule", | ||
"version": "1.0.0", | ||
"main": "index.js" | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we linkify this please? i.e.
[https://github.com/Microsoft/TypeScript/issues/12358](https://github.com/Microsoft/TypeScript/issues/12358)
I think