Skip to content

Commit 0bef5aa

Browse files
hanslhansl
committed
fix(@ngtools/webpack): fix resolution fallback of paths-plugin
If there is a package.json we should also verify that it has a main or fesm field to see if webpack would actually resolve it properly. Otherwise use the JavaScript resolution. This is a temporary fix and still has obvious limitations and issues. Namely, this code is never run if there is only one path mapping, but that falls outside the scope of this fix. Also, some people might have valid packages but want to resolve to the JS file itself (which is what TypeScript does by default). These should be fixed with a refactoring of the path plugin. Fixes NativeScript/nativescript-dev-webpack#607
1 parent 91e7c3e commit 0bef5aa

File tree

14 files changed

+227
-8
lines changed

14 files changed

+227
-8
lines changed

‎packages/ngtools/webpack/src/paths-plugin.ts

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,15 +160,40 @@ export function resolveWithPaths(
160160
const jsFilePath = `${pathNoExtension}.js`;
161161

162162
if (host.fileExists(pathNoExtension)) {
163-
// This is mainly for secondary entry points
164-
// ex: 'node_modules/@angular/core/testing.d.ts' -> 'node_modules/@angular/core/testing'
165-
request.request = pathNoExtension;
166-
} else if (host.fileExists(packageRootPath)) {
167-
// Let webpack resolve the correct module format
168-
request.request = pathDirName;
169-
} else if (host.fileExists(jsFilePath)) {
163+
// This is mainly for secondary entry points
164+
// ex: 'node_modules/@angular/core/testing.d.ts' -> 'node_modules/@angular/core/testing'
165+
request.request = pathNoExtension;
166+
} else {
167+
const packageJsonContent = host.readFile(packageRootPath);
168+
let newRequest: string | undefined;
169+
170+
if (packageJsonContent) {
171+
try {
172+
const packageJson = JSON.parse(packageJsonContent);
173+
174+
// Let webpack resolve the correct module format IIF there is a module resolution field
175+
// in the package.json. These are all official fields that Angular uses.
176+
if (typeof packageJson.main == 'string'
177+
|| typeof packageJson.browser == 'string'
178+
|| typeof packageJson.module == 'string'
179+
|| typeof packageJson.es2015 == 'string'
180+
|| typeof packageJson.fesm5 == 'string'
181+
|| typeof packageJson.fesm2015 == 'string') {
182+
newRequest = pathDirName;
183+
}
184+
} catch {
185+
// Ignore exceptions and let it fall through (ie. if package.json file is invalid).
186+
}
187+
}
188+
189+
if (newRequest === undefined && host.fileExists(jsFilePath)) {
170190
// Otherwise, if there is a file with a .js extension use that
171-
request.request = jsFilePath;
191+
newRequest = jsFilePath;
192+
}
193+
194+
if (newRequest !== undefined) {
195+
request.request = newRequest;
196+
}
172197
}
173198

174199
callback(null, request);

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/aotplugin.config.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"tsConfigPath": "./src/tsconfig.json",
3+
"mainPath": "app/main.jit.ts"
4+
}

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/package.json

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
{
2+
"name": "test",
3+
"license": "MIT",
4+
"dependencies": {
5+
"@angular/common": "^5.0.0-rc.8",
6+
"@angular/compiler": "^5.0.0-rc.8",
7+
"@angular/compiler-cli": "^5.0.0-rc.8",
8+
"@angular/core": "^5.0.0-rc.8",
9+
"@angular/http": "^5.0.0-rc.8",
10+
"@angular/platform-browser": "^5.0.0-rc.8",
11+
"@angular/platform-browser-dynamic": "^5.0.0-rc.8",
12+
"@angular/platform-server": "^5.0.0-rc.8",
13+
"@angular/router": "^5.0.0-rc.8",
14+
"@ngtools/webpack": "0.0.0",
15+
"core-js": "^2.4.1",
16+
"rxjs": "^5.5.0",
17+
"zone.js": "^0.8.14"
18+
},
19+
"devDependencies": {
20+
"node-sass": "^4.7.0",
21+
"performance-now": "^0.2.0",
22+
"preprocess-loader": "^0.2.2",
23+
"raw-loader": "^0.5.1",
24+
"sass-loader": "^6.0.0",
25+
"typescript": "~2.4.2",
26+
"webpack": "~4.0.1",
27+
"webpack-cli": "~2.0.9"
28+
}
29+
}

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/src/app/app.module.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
import { Component, NgModule } from '@angular/core';
9+
import { BrowserModule } from '@angular/platform-browser';
10+
import { Foo } from 'foo/foo';
11+
12+
console.log(Foo); // Make sure it's used.
13+
14+
@Component({
15+
selector: 'home-view',
16+
template: 'home!',
17+
})
18+
export class HomeView {}
19+
20+
@NgModule({
21+
declarations: [
22+
HomeView,
23+
],
24+
imports: [
25+
BrowserModule,
26+
],
27+
bootstrap: [HomeView],
28+
})
29+
export class AppModule { }

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/src/app/main.jit.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import 'core-js/es7/reflect';
2+
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
3+
import {AppModule} from './app.module';
4+
5+
platformBrowserDynamic().bootstrapModule(AppModule);

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/src/foo/foo.d.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
export class Foo {}

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/src/foo/foo.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
console.log('NGTOOLS_WEBPACK_TEST_WRONG_FILE');
3+
4+
module.exports = {
5+
Foo: () => {}, // Empty "class".
6+
};

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/src/foo/package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"main": "right/foo.js"
3+
}

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/src/foo/right/foo.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
console.log('NGTOOLS_WEBPACK_TEST_RIGHT_FILE');
3+
4+
module.exports = {
5+
Foo: () => {}, // Empty "class".
6+
};

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/src/index.html

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<html>
2+
<head>
3+
<meta charset="UTF-8">
4+
<title>Document</title>
5+
<base href="">
6+
</head>
7+
<body>
8+
<app-root></app-root>
9+
<script src="node_modules/zone.js/dist/zone.js"></script>
10+
<script src="dist/app.main.js"></script>
11+
</body>
12+
</html>

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/src/tsconfig.json

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
{
2+
"compilerOptions": {
3+
"module": "es2015",
4+
"moduleResolution": "node",
5+
"target": "es5",
6+
"baseUrl": ".",
7+
"noImplicitAny": false,
8+
"sourceMap": true,
9+
"mapRoot": "",
10+
"emitDecoratorMetadata": true,
11+
"experimentalDecorators": true,
12+
"lib": [
13+
"es2017",
14+
"dom"
15+
],
16+
"outDir": "lib",
17+
"skipLibCheck": true,
18+
"rootDir": ".",
19+
"paths": {
20+
"foo/*": [
21+
"./foo/*",
22+
"*"
23+
]
24+
}
25+
},
26+
"angularCompilerOptions": {
27+
"genDir": "app/generated/",
28+
"entryModule": "app/app.module#AppModule"
29+
}
30+
}

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/webpack.config.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
const ngToolsWebpack = require('@ngtools/webpack');
9+
const path = require('path');
10+
11+
const flags = require('./webpack.flags.json');
12+
13+
const preprocessLoader = 'preprocess-loader' + (flags.DEBUG ? '?+DEBUG' : '');
14+
15+
16+
module.exports = {
17+
resolve: {
18+
extensions: ['.ts', '.js']
19+
},
20+
entry: './src/app/main.jit.ts',
21+
output: {
22+
path: path.resolve('./dist'),
23+
publicPath: 'dist/',
24+
filename: 'app.main.js'
25+
},
26+
plugins: [
27+
new ngToolsWebpack.AngularCompilerPlugin(require('./aotplugin.config.json'))
28+
],
29+
module: {
30+
rules: [
31+
{ test: /\.scss$/, loaders: ['raw-loader', 'sass-loader', preprocessLoader] },
32+
{ test: /\.css$/, loader: 'raw-loader' },
33+
{ test: /\.html$/, loaders: ['raw-loader', preprocessLoader] },
34+
// Use preprocess to remove DEBUG only code.
35+
// @ngtools/webpack must be the first (right most) loader.
36+
{ test: /\.ts$/, use: [
37+
{ loader: preprocessLoader },
38+
{ loader: '@ngtools/webpack' }
39+
] }
40+
]
41+
},
42+
devServer: {
43+
historyApiFallback: true
44+
}
45+
};

‎tests/legacy-cli/e2e/assets/webpack/test-app-path-mapping/webpack.flags.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"DEBUG": false
3+
}

‎tests/legacy-cli/e2e/tests/packages/webpack/test-path-mapping.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import {normalize} from 'path';
2+
import {createProjectFromAsset} from '../../../utils/assets';
3+
import {exec} from '../../../utils/process';
4+
import {expectFileSizeToBeUnder, replaceInFile, expectFileToMatch} from '../../../utils/fs';
5+
6+
7+
export default function(skipCleaning: () => void) {
8+
return Promise.resolve()
9+
.then(() => createProjectFromAsset('webpack/test-app-path-mapping'))
10+
.then(() => exec(normalize('node_modules/.bin/webpack-cli')))
11+
.then(() => expectFileToMatch('dist/app.main.js', 'NGTOOLS_WEBPACK_TEST_RIGHT_FILE'))
12+
.then(() => skipCleaning());
13+
}

0 commit comments

Comments
 (0)