search

plotly / dash-component-boilerplate

Get started creating your own Dash components here.
269 stars 182 forks source link

Updating scripts to match dash-core-components #20

Open valentijnnieman opened 5 years ago

valentijnnieman commented 5 years ago

We should update the scripts in package.json the same way as we did for dash-core-components. However, I don't think we should force prettier on the users, since some people are not a fan (although I noticed it's pretty much set up already in this repo, except there's no prettier package in package.json's dependencies. If we do want it, we could use the format and format:test scripts). We should copy over the publish script however, or perhaps add it to a scripts repo as mentioned in #17. We should also copy the build:watch script, which rebuilds the bundle on src/ files changes, the 'lint and test scripts are also nice to have here.

T4rk1n commented 5 years ago

I personally hates prettier and do not want to see it here forced on users. I think the formatted code (with the dcc configs) is actually uglier, less readable (if you already have sane line length) and harder to refactor IMO (splitting lines makes them harder to move around with my IDE ctrl+shift selection).

As for the publish script, what does it saves ? like one command line instead of three, I don't think it's that useful. Also, you don't have to publish on npm if you don't want to, so the script would have to be adapted to take that variable into consideration.

The build:watch , lint, test scripts should be added.

valentijnnieman commented 5 years ago

Like I said I agree that prettier shouldn't be forced (so perhaps we should remove the .prettierrc and other prettier config?) the publish script is handy IMO if you've never published before and just want to run a single command without having to read up on how to use twine etc. I was also under the impression that the JS source code was downloaded by Dash (or dash-renderer?) from NPM so every component would need it's JS code published on NPM?

T4rk1n commented 5 years ago

They don't need to be uploaded to NPM, I discovered that you only need to remove the external_url from the _js_dist to force dash to serve locally, even if app.scripts.config.serve_locally=False. https://github.com/plotly/dash/issues/405

So I added a publish_on_npm cookiecutter variable to disable it if false.

valentijnnieman commented 5 years ago

Oh nice! In any case, it could still be useful to have a simple script that does twine upload dist/component_version_0.0.1.tar.gz or something for you, which is what that publish script in DCC does. It might not be all that useful to you, but for newcomers it could be useful.

valentijnnieman commented 5 years ago

Just linking to https://github.com/plotly/dash-core-components/pull/299#issuecomment-422016529 where we discussed making the same improvements to these repo's too, in case that wasn't clear. Haven't had time to make those improvements yet, so future me if you're reading this, this is on you!

valentijnnieman commented 5 years ago

But seriously if anyone else feels like picking this up, that would be great!