Merge pull request #6440 from plotly/false-umdNamedDefine

archmoj · web-flow · commit 360213e73deb · 2023-01-18T16:52:37.000-05:00
Fix library's imported name using `requirejs` AMD loader
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -400,6 +400,9 @@ jobs:
       - run:
           name: Test plot-schema.json diff - If failed, after (npm start) you could run (npm run schema && git add test/plot-schema.json && git commit -m "update plot-schema diff")
           command: diff --unified --color dist/plot-schema.json test/plot-schema.json
+      - run:
+          name: Test plotly.min.js import using amdefine
+          command: npm run test-amdefine
       - run:
           name: Test plotly.min.js import using requirejs
           command: npm run test-requirejs
diff --git a/.eslintignore b/.eslintignore
@@ -3,5 +3,6 @@ node_modules
 dist
 build
 
+tasks/test_amdefine.js
 tasks/test_requirejs.js
 test/jasmine/assets/jquery-1.8.3.min.js
diff --git a/devtools/test_dashboard/server.js b/devtools/test_dashboard/server.js
@@ -37,8 +37,7 @@ devtoolsConfig.output = {
     filename: 'test_dashboard-bundle.js',
     library: {
         name: 'Tabs',
-        type: 'umd',
-        umdNamedDefine: true
+        type: 'umd'
     }
 };
 
diff --git a/draftlogs/6440_fix.md b/draftlogs/6440_fix.md
@@ -0,0 +1 @@
+ - Fix library's imported name using `requirejs` AMD loader (regression introduced in 2.17.0) [[#6440](https://github.com/plotly/plotly.js/pull/6440)]
diff --git a/package.json b/package.json
@@ -48,6 +48,7 @@
     "test-export": "node test/image/export_test.js",
     "test-syntax": "node tasks/test_syntax.js && npm run find-strings -- --no-output",
     "test-bundle": "node tasks/test_bundle.js",
+    "test-amdefine": "node tasks/test_amdefine.js",
     "test-requirejs": "node tasks/test_requirejs.js",
     "test-plain-obj": "node tasks/test_plain_obj.js",
     "test": "npm run test-jasmine -- --nowatch && npm run test-bundle && npm run test-image && npm run test-export && npm run test-syntax && npm run lint",
diff --git a/tasks/test_amdefine.js b/tasks/test_amdefine.js
@@ -0,0 +1,28 @@
+var JSDOM = require('jsdom').JSDOM;
+global.document = new JSDOM('<!DOCTYPE html><head></head><html><body></body></html>').window.document;
+global.window = document.defaultView;
+global.window.document = global.document;
+global.self = global.window;
+global.Blob = global.window.Blob;
+global.DOMParser = global.window.DOMParser;
+global.getComputedStyle = global.window.getComputedStyle;
+global.window.URL.createObjectURL = function() {};
+
+// see: Building node modules with AMD or RequireJS https://requirejs.org/docs/node.html
+if(typeof define !== 'function') {
+    var define = require('amdefine')(module);
+}
+
+define(function(require) {
+    var plotly = require('../dist/plotly.min.js');
+
+    if(plotly) {
+        console.log(plotly);
+    } else {
+        throw 'Error: loading with amdefine';
+    }
+
+    // The value returned from the function is
+    // used as the module export visible to Node.
+    return function() {};
+});
diff --git a/tasks/test_requirejs.js b/tasks/test_requirejs.js
@@ -8,21 +8,19 @@ global.DOMParser = global.window.DOMParser;
 global.getComputedStyle = global.window.getComputedStyle;
 global.window.URL.createObjectURL = function() {};
 
-// see: Building node modules with AMD or RequireJS https://requirejs.org/docs/node.html
-if(typeof define !== 'function') {
-    var define = require('amdefine')(module);
-}
+var requirejs = require('requirejs');
 
-define(function(require) {
-    var plotly = require('../dist/plotly.min.js');
+requirejs.config({
+    paths: {
+        'plotly': '../dist/plotly.min'
+    }
+});
 
+requirejs(['plotly'],
+function(plotly) {
     if(plotly) {
         console.log(plotly);
     } else {
         throw 'Error: loading with requirejs';
     }
-
-    // The value returned from the function is
-    // used as the module export visible to Node.
-    return function() {};
 });
diff --git a/test/jasmine/karma.conf.js b/test/jasmine/karma.conf.js
@@ -5,6 +5,7 @@ var minimist = require('minimist');
 var NodePolyfillPlugin = require('node-polyfill-webpack-plugin');
 var LoaderOptionsPlugin = require('webpack').LoaderOptionsPlugin;
 var constants = require('../../tasks/util/constants');
+var webpackConfig = require('../../webpack.config.js');
 
 var isCI = Boolean(process.env.CI);
 
@@ -298,11 +299,7 @@ func.defaultConfig = {
             new LoaderOptionsPlugin({
                 // test: /\.xxx$/, // may apply this only for some modules
                 options: {
-                    library: {
-                        name: 'Plotly',
-                        type: 'umd',
-                        umdNamedDefine: true
-                    }
+                    library: webpackConfig.output.library
                 }
             })
         ]
diff --git a/webpack.config.js b/webpack.config.js
@@ -10,7 +10,7 @@ module.exports = {
         library: {
             name: 'Plotly',
             type: 'umd',
-            umdNamedDefine: true
+            umdNamedDefine: false
         }
     },
     module: {

Original file line number	Diff line number	Diff line change
`@@ -37,8 +37,7 @@ devtoolsConfig.output = {`
`37`	`37`	`filename: 'test_dashboard-bundle.js',`
`38`	`38`	`library: {`
`39`	`39`	`name: 'Tabs',`
`40`		`- type: 'umd',`
`41`		`- umdNamedDefine: true`
	`40`	`+ type: 'umd'`
`42`	`41`	`}`
`43`	`42`	`};`
`44`	`43`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+ - Fix library's imported name using `requirejs` AMD loader (regression introduced in 2.17.0) [[#6440](https://github.com/plotly/plotly.js/pull/6440)]
Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,7 @@ module.exports = {`
`10`	`10`	`library: {`
`11`	`11`	`name: 'Plotly',`
`12`	`12`	`type: 'umd',`
`13`		`- umdNamedDefine: true`
	`13`	`+ umdNamedDefine: false`
`14`	`14`	`}`
`15`	`15`	`},`
`16`	`16`	`module: {`