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

Fix library's imported name using requirejs AMD loader #6440

Merged
merged 7 commits into from Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .circleci/config.yml
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions .eslintignore
Expand Up @@ -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
3 changes: 1 addition & 2 deletions devtools/test_dashboard/server.js
Expand Up @@ -37,8 +37,7 @@ devtoolsConfig.output = {
filename: 'test_dashboard-bundle.js',
library: {
name: 'Tabs',
type: 'umd',
umdNamedDefine: true
type: 'umd'
}
};

Expand Down
1 change: 1 addition & 0 deletions 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)]
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -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",
Expand Down
28 changes: 28 additions & 0 deletions 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() {};
});
18 changes: 8 additions & 10 deletions tasks/test_requirejs.js
Expand Up @@ -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() {};
});
7 changes: 2 additions & 5 deletions test/jasmine/karma.conf.js
Expand Up @@ -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);

Expand Down Expand Up @@ -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
}
})
]
Expand Down
2 changes: 1 addition & 1 deletion webpack.config.js
Expand Up @@ -10,7 +10,7 @@ module.exports = {
library: {
name: 'Plotly',
type: 'umd',
umdNamedDefine: true
umdNamedDefine: false
}
},
module: {
Expand Down