From d3e2ea19ce77044de57d077eff276e4959b299f5 Mon Sep 17 00:00:00 2001 From: Thomas Sileghem Date: Wed, 19 Jul 2017 21:50:58 +0100 Subject: [PATCH] fix: use correct module when using multiple modules versions (#25) --- src/index.js | 28 ++++++-- test/core.js | 35 ++++++++++ test/fixtures/app/dir/single.js | 1 + test/fixtures/app/mix.js | 2 + test/fixtures/app/node_modules/a/a.js | 2 + .../a/node_modules/react/package.json | 68 +++++++++++++++++++ .../a/node_modules/react/react.js | 1 + 7 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 test/fixtures/app/dir/single.js create mode 100644 test/fixtures/app/node_modules/a/node_modules/react/package.json create mode 100644 test/fixtures/app/node_modules/a/node_modules/react/react.js diff --git a/src/index.js b/src/index.js index 4ccf3b0..f7624a3 100644 --- a/src/index.js +++ b/src/index.js @@ -21,10 +21,11 @@ export default class ModulesCdnWebpackPlugin { this.disable = disable; this.env = env || process.env.NODE_ENV || 'development'; - this.urls = {}; this.exclude = exclude || []; this.only = only || null; this.verbose = verbose === true; + + this.modulesFromCdn = {}; } apply(compiler) { @@ -72,6 +73,16 @@ export default class ModulesCdnWebpackPlugin { const {version, peerDependencies} = readPkgUp({cwd: resolvePkg(modulePath, {cwd: contextPath})}).pkg; + const isModuleAlreadyLoaded = Boolean(this.modulesFromCdn[modulePath]); + if (isModuleAlreadyLoaded) { + const isSameVersion = this.modulesFromCdn[modulePath].version === version; + if (isSameVersion) { + return modulePath; + } + + return false; + } + const cdnConfig = moduleToCdn(modulePath, version, {env}); if (cdnConfig == null) { @@ -93,7 +104,12 @@ export default class ModulesCdnWebpackPlugin { } } - this.urls[cdnConfig.name] = cdnConfig.url; + // TODO: on next breaking change, rely on module-to-cdn>=3.1.0 to get version + this.modulesFromCdn[modulePath] = Object.assign( + {}, + cdnConfig, + {version} + ); return cdnConfig.var; } @@ -103,11 +119,11 @@ export default class ModulesCdnWebpackPlugin { const entrypoint = compilation.entrypoints[Object.keys(compilation.entrypoints)[0]]; const parentChunk = entrypoint.chunks.find(x => x.isInitial()); - for (const name of Object.keys(this.urls)) { - const url = this.urls[name]; + for (const name of Object.keys(this.modulesFromCdn)) { + const cdnConfig = this.modulesFromCdn[name]; const chunk = compilation.addChunk(name); - chunk.files.push(url); + chunk.files.push(cdnConfig.url); chunk.parents = [parentChunk]; parentChunk.addChunk(chunk); @@ -128,7 +144,7 @@ export default class ModulesCdnWebpackPlugin { includeAssetsPlugin.apply(compiler); compiler.plugin('after-compile', (compilation, cb) => { - const assets = Object.values(this.urls); + const assets = Object.keys(this.modulesFromCdn).map(key => this.modulesFromCdn[key].url); // HACK: Calling the constructor directly is not recomended // But that's the only secure way to edit `assets` afterhand diff --git a/test/core.js b/test/core.js index 53be913..eab9c43 100644 --- a/test/core.js +++ b/test/core.js @@ -391,3 +391,38 @@ test('async loading', async t => { const doesIncludeReact = outputs.some(output => includes(output, 'THIS IS REACT!')); t.false(doesIncludeReact); }); + +test('when using multiple versions of a module, make sure the right version is used for each', async t => { + await cleanDir(path.resolve(__dirname, './fixtures/output/multiple-versions')); + + const stats = await runWebpack({ + context: path.resolve(__dirname, './fixtures/app'), + + output: { + publicPath: '', + path: path.resolve(__dirname, './fixtures/output/multiple-versions') + }, + + entry: { + app: './mix.js' + }, + + plugins: [ + new ModulesCdnWebpackPlugin() + ] + }); + + const files = stats.compilation.chunks.reduce((files, x) => files.concat(x.files), []); + + t.true(includes(files, 'app.js')); + t.true(includes(files, 'https://unpkg.com/react@15.6.1/dist/react.js')); + + const output = await fs.readFile(path.resolve(__dirname, './fixtures/output/multiple-versions/app.js')); + + // NOTE: not inside t.false to prevent ava to display whole file in console + const doesIncludeReact14 = includes(output, 'THIS IS REACT@0.14.9!'); + t.true(doesIncludeReact14); + + const doesIncludeReact = includes(output, 'THIS IS REACT!'); + t.false(doesIncludeReact); +}); diff --git a/test/fixtures/app/dir/single.js b/test/fixtures/app/dir/single.js new file mode 100644 index 0000000..aaf4046 --- /dev/null +++ b/test/fixtures/app/dir/single.js @@ -0,0 +1 @@ +import React from 'react'; diff --git a/test/fixtures/app/mix.js b/test/fixtures/app/mix.js index be7e2dd..20b8dc8 100644 --- a/test/fixtures/app/mix.js +++ b/test/fixtures/app/mix.js @@ -1,2 +1,4 @@ import a from 'a'; import React from 'react'; + +import single from './dir/single'; diff --git a/test/fixtures/app/node_modules/a/a.js b/test/fixtures/app/node_modules/a/a.js index d8326ac..554db1c 100644 --- a/test/fixtures/app/node_modules/a/a.js +++ b/test/fixtures/app/node_modules/a/a.js @@ -1 +1,3 @@ +import React from 'react'; + console.log('THIS IS A!'); diff --git a/test/fixtures/app/node_modules/a/node_modules/react/package.json b/test/fixtures/app/node_modules/a/node_modules/react/package.json new file mode 100644 index 0000000..be720f4 --- /dev/null +++ b/test/fixtures/app/node_modules/a/node_modules/react/package.json @@ -0,0 +1,68 @@ +{ + "_from": "react", + "_id": "react@0.14.9", + "_inBundle": false, + "_integrity": "sha1-uqhDTsZ4C96ZfNw4C3nNM7ljk98=", + "_location": "/react", + "_phantomChildren": {}, + "_requested": { + "type": "tag", + "registry": true, + "raw": "react", + "name": "react", + "escapedName": "react", + "rawSpec": "", + "saveSpec": null, + "fetchSpec": "latest" + }, + "_requiredBy": [ + "#USER", + "/" + ], + "_resolved": "https://registry.npmjs.org/react/-/react-0.14.9.tgz", + "_shasum": "baa8434ec6780bde997cdc380b79cd33b96393df", + "_spec": "react", + "_where": "/home/thomas/Code/modules-cdn-webpack-plugin/test/fixtures/multiple", + "browserify": { + "transform": [ + "loose-envify" + ] + }, + "bugs": { + "url": "https://github.com/facebook/react/issues" + }, + "bundleDependencies": false, + "dependencies": { + "create-react-class": "^15.6.0", + "fbjs": "^0.8.9", + "loose-envify": "^1.1.0", + "object-assign": "^4.1.0", + "prop-types": "^15.5.10" + }, + "deprecated": false, + "description": "React is a JavaScript library for building user interfaces.", + "engines": { + "node": ">=0.10.0" + }, + "files": [ + "LICENSE", + "PATENTS", + "addons.js", + "react.js", + "addons/", + "dist/", + "lib/" + ], + "homepage": "https://facebook.github.io/react/", + "keywords": [ + "react" + ], + "license": "BSD-3-Clause", + "main": "react.js", + "name": "react", + "repository": { + "type": "git", + "url": "git+https://github.com/facebook/react.git" + }, + "version": "0.14.9" +} diff --git a/test/fixtures/app/node_modules/a/node_modules/react/react.js b/test/fixtures/app/node_modules/a/node_modules/react/react.js new file mode 100644 index 0000000..95a0187 --- /dev/null +++ b/test/fixtures/app/node_modules/a/node_modules/react/react.js @@ -0,0 +1 @@ +console.log('THIS IS REACT@0.14.9!');