dash-component-boilerplate icon indicating copy to clipboard operation
dash-component-boilerplate copied to clipboard

Improve JS facing packaging

Open Marc-Andre-Rivet opened this issue 7 years ago • 9 comments

Ran into some issues packaging the dash-table for JS consumption and would like to suggest making these modifications to the boilerplate in order to make the npm packages cleaner.

  1. Generate the package.json "main" property to be
  "main": {{cookiecutter.project_shortname}}/bundle.js

This makes import MyComponent from 'my-component' return the expected value (with libraryTarget='umd', most probably the component itself)

  1. Do away with blacklisting through .npmignore, use whitelisting instead through package.json "files" property

Default value could be

  "files": [
    "/{{cookiecutter.project_shortname}}/bundle*.js"
  ],

This would cover future bundle.dev.js, bundle.min.js, etc. Package.json and *.md files are picked up automatically by npm.

  1. Modify the build:py script command to copy package.json into another file name (e.g. bundle.json) + update impacted setup.py and init.py files

This allows npm publish to pick up the file in {{cookiecutter.project_shortname}} correctly (otherwise since a package.json is present, the folder's content will be ignored -- configuring .npmignore to ignore that package.json will not modify this behavior)

  1. Do not promote the use of shrinkwrap for fully self-contained builds -- the shrinkwrap will override the peer & dev dependencies behavior and will install on wrapped dependencies on npm install

  2. Make it optional to set libraryTarget to 'umd' (maybe this is too specific to dash-table)

output: {
    libraryTarget: 'umd'
}

And modify react and react-dom external dependency resolution to cover the various use cases

        externals: {
            react: {
                commonjs: "react",
                commonjs2: "react",
                amd: "React",
                root: "React"
            },
            "react-dom": {
                commonjs: "react-dom",
                commonjs2: "react-dom",
                amd: "ReactDOM",
                root: "ReactDOM"
            }
        }

Marc-Andre-Rivet avatar Nov 09 '18 14:11 Marc-Andre-Rivet

Re point 3, this would impact only __init__.py not setup.py, and it makes a lot of sense to me, although I'd call it package-info.json or something other than bundle.json. We could implement this change quite quickly by tweaking #33 which is still open atm.

nicolaskruchten avatar Nov 09 '18 15:11 nicolaskruchten

Do away with blacklisting through .npmignore, use whitelisting instead through package.json "files" property

I think a blacklist is more appropriate for a boilerplate, if someone wants to adds more js/css files, they already have to modify the appropriate _dist in __init__ which is validated by the _validate_init.py. It would just make another place that risks failing and need more documentations/validations.

T4rk1n avatar Nov 12 '18 17:11 T4rk1n

One thing to note is that there's not really a strong reason to publish Dash components on the NPM registry as far as I know, other than just making them available to a CDN like unpkg. What's the use-case for a non-Dash JS project to depend on a dash-component NPM module?

nicolaskruchten avatar Nov 12 '18 20:11 nicolaskruchten

(and by the by, I think that serving the components locally by default makes more sense than relying on a CDN by default, thereby even further removing the need for NPM-publishing Dash components ;)

nicolaskruchten avatar Nov 12 '18 20:11 nicolaskruchten

https://github.com/plotly/dash/issues/284

nicolaskruchten avatar Nov 12 '18 20:11 nicolaskruchten

@nicolaskruchten So set publish on npm to false by default ?

T4rk1n avatar Nov 12 '18 20:11 T4rk1n

hmmm good question. I would tend to vote yes personally, as it's not really required if serving locally is the default.

nicolaskruchten avatar Nov 12 '18 21:11 nicolaskruchten

@nicolaskruchten I agree with you that for most components npm publish seems mostly to be for the CDN. The dash-table is meant to be useable in a non-Dash environment but that's the exception more than the norm at the moment -- and if that's the case it makes some of the points above moot.

Marc-Andre-Rivet avatar Nov 12 '18 21:11 Marc-Andre-Rivet

Yes it does seem that dash-table is the project with the unique requirements in this context.

nicolaskruchten avatar Nov 12 '18 23:11 nicolaskruchten