Skip to content

add minimum ify-loader version to readme #2011

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

Merged
merged 1 commit into from
Sep 18, 2017

Conversation

bpostlethwaite
Copy link
Member

To account for recent fix found here: browserify/ify-loader#5 (comment)

@bpostlethwaite bpostlethwaite force-pushed the update-readme-with-ify-loader-version branch 2 times, most recently from 8c186c6 to 83e1331 Compare September 18, 2017 14:24
README.md Outdated
@@ -111,7 +111,7 @@ Important: the plotly.js code base contains some non-ascii characters. Therefore

#### Webpack Usage with Modules

Browserify [transforms](https://github.com/substack/browserify-handbook#transforms) are required to build plotly.js, namely, [glslify](https://github.com/stackgl/glslify) to transform WebGL shaders and [cwise](https://github.com/scijs/cwise) to compile component-wise array operations. To make the trace module system work with Webpack, you will need to install [ify-loader](https://github.com/hughsk/ify-loader) and add it to your `webpack.config.json` for your build to correctly bundle plotly.js files.
Browserify [transforms](https://github.com/substack/browserify-handbook#transforms) are required to build plotly.js, namely, [glslify](https://github.com/stackgl/glslify) to transform WebGL shaders and [cwise](https://github.com/scijs/cwise) to compile component-wise array operations. To make the trace module system work with Webpack, you will need to install [ify-loader@v1.1.0+](https://github.com/hughsk/ify-loader) and add it to your `webpack.config.json` for your build to correctly bundle plotly.js files.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal, but appears brfs is also used, unless I'm mistaken. Though that module is deprecated anyway. Also, thought brfs would show up in more places.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do users wanting to build plotly.js need to explicitly add brfs?

Copy link
Contributor

@rreusser rreusser Sep 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the best of my knowledge, they don't need to add any of the transforms explicitly, do they? It's picked up in the dev dependencies and package.json of the corresponding module, right? Like you don't have to specify anything about glslify, for one, to get ify-loader to use glslify when building plotly.js, I think…? Correct me if I'm wrong.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's correct. I think we should simplify this section. We don't need to mention any of this stuff other than "use ify-loader" and provide a simple webpack config example.

Copy link
Contributor

@rreusser rreusser Sep 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, here's what I was using as a minimal example:

// index.js:
var Plotly = require('plotly.js');

And the webpack.config.js:

module.exports = {
    entry: "./index.js",
    output: {
        path: __dirname,
        filename: "plotly-bundle.js"
    },
    module: {
        loaders: [{
            test: /\.js$/,
            loader: 'ify-loader'
        }]
    }
};

Maybe just suggest the loaders section?

@bpostlethwaite bpostlethwaite force-pushed the update-readme-with-ify-loader-version branch from 83e1331 to 95dd9c0 Compare September 18, 2017 17:36
README.md Outdated
For plotly.js to build with Webpack you will need to install [[email protected]+](https://github.com/hughsk/ify-loader) and add it to your `webpack.config.json`. This adds Browserify transform compatibility to Webpack which is necessary for some plotly.js dependencies.

A repo that demonstrates how to build plotly.js with Webpack can be found [here](https://github.com/rreusser/plotly-webpack). In short add `ify-loader` to the `module` section in your `webpack.config.js`:
```json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/json/js/

@bpostlethwaite bpostlethwaite force-pushed the update-readme-with-ify-loader-version branch from 95dd9c0 to 3430078 Compare September 18, 2017 17:37
@etpinard
Copy link
Contributor

LGTM 💃

@bpostlethwaite bpostlethwaite merged commit 790d470 into master Sep 18, 2017
@bpostlethwaite bpostlethwaite deleted the update-readme-with-ify-loader-version branch September 18, 2017 19:54
@rreusser
Copy link
Contributor

I've fixed up my instructions to turn this from an issue-demonstration repo into more of a webpack-howto repo: https://github.com/rreusser/plotly-webpack

PRs for clarity welcome, though it's pretty simple as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants