search

plotly / dash-renderer

OBSOLETE has been merged into dash
https://github.com/plotly/dash
MIT License
97 stars 32 forks source link

Move check-prop-types to dependencies #176

Closed ajacksified closed 5 years ago

ajacksified commented 5 years ago

It's used by TreeContainer: https://github.com/plotly/dash-renderer/blob/ab4d2d8379910eaa31c9d1091d72946b4dc087d3/src/TreeContainer.js#L26

I ran into this because check-prop-types is devDependency being used as a normal dependency, and devDependencies aren't installed when you install dash-renderer into node_modules. I'm using the src because there's no node-compatible pre-built file (the files in dash_renderer pollute window, which is no good.) It's generally good practice for npm modules to include a pre-built file that exports the code as an importable rather than putting an object on window, so it can be used in other projects that use commonjs (like anything using webpack or browserify.) I can make another PR to include that if you're interested.

As an aside: also adding .txt to your CSS files is another thing that gave me build issues; I had to write in special rules. Is there a particular reason for that? https://github.com/plotly/dash-renderer/blob/ab4d2d8379910eaa31c9d1091d72946b4dc087d3/src/components/error/werkzeug.css.txt

alexcjohnson commented 5 years ago

Thanks @ajacksified - you're absolutely right. But this repo has been merged into https://github.com/plotly/dash - can you remake this PR over there?

byronz commented 5 years ago

Hi @ajacksified
thanks for the PR, this repo is deprecated though. FYI, the dash-renderer project is now part of main dash repo https://github.com/plotly/dash you can create another PR there, we would happy to accept the change.