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

Publish `plotly.js-locales` to npm #3223

Merged
merged 3 commits into from Nov 13, 2018

Conversation

Projects
None yet
2 participants
@etpinard
Copy link
Member

commented Nov 7, 2018

resolves #3083, a continuation from #2670 (the partial-bundle-pkgs PR)

As locale files are fairly small, I decided to publish only one plotly.js-locales package (as opposed to one per locale) and ask users (in the plotly.js-locales README) to grab to relevant locale as:

// ES6 module
import Plotly from 'plotly.js';
import locale from 'plotly.js-locales/fr';

// CommonJS
var Plotly = require('plotly.js');
var locale = require('plotly.js-locales/fr');

// then
Plotly.register(locale);

that is, users would npm-download all the locales, but only bundle the locale(s) they need.

Reviewers can look at:

publish-locales...publish-locales-build

to see to generated git-ignored files in build/.

cc @plotly/plotly_js and @nicolaskruchten who may find this useful for RCE and dash users.

etpinard added some commits Nov 7, 2018

@@ -205,6 +205,7 @@ function syncLocalesPkg(d) {
'',
'// then',
'Plotly.register(locale);',
'Plotly.setPlotConfig({locale: \'fr\'})',

This comment has been minimized.

Copy link
@alexcjohnson

alexcjohnson Nov 13, 2018

Contributor

Good call 👍

@alexcjohnson

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2018

@etpinard this looks great! Assuming you've verified that the built files work as described in the README (and they certainly look like they will) 💃

@etpinard

This comment has been minimized.

Copy link
Member Author

commented Nov 13, 2018

Assuming you've verified that the built files work as described

Verified

@etpinard etpinard merged commit ad937d2 into master Nov 13, 2018

7 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: publish Your tests passed on CircleCI!
Details
ci/circleci: test-image Your tests passed on CircleCI!
Details
ci/circleci: test-image2 Your tests passed on CircleCI!
Details
ci/circleci: test-jasmine Your tests passed on CircleCI!
Details
ci/circleci: test-jasmine2 Your tests passed on CircleCI!
Details
ci/circleci: test-syntax Your tests passed on CircleCI!
Details

@etpinard etpinard deleted the publish-locales branch Nov 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.