sphinx_rtd_theme
sphinx_rtd_theme copied to clipboard
readthedocs
npm: Split devDependencies into regular dependencies
This is correct usage and allows developers to easily see what affets the built package and what affects the devOps pipeline.
Technically the theme.js requires jquary however reading npm docs I think this pr is still correct usage. Splitting them still shows what affects built assets an what does not.
https://docs.npmjs.com/specifying-dependencies-and-devdependencies-in-a-package-json-file specifies dev requirements as dependencs needed for local development and testing.
If we update fontawesome, it is a required change to the product, updating webpack does not affect the the end product.
I dont think npm is super strict in this regard but I prefer this organization between what is needed to make the theme and what is needed to actually build the theme.
There are a few arguments for both styles, however npm and the JS community don't have a consistent answer here. I sort of get the argument for putting them in dependencies, but this is definitely where there is some conflict because constructs used for node application code are being reused for node front end packaging.
Vue and React do what I'm describing: anything that is included in our bundle or used for producing the bundle is in devDependencies, and anything that is directly linked by our bundle.
The main reason I don't think this is necessary is that upgrading bourbon, wyrm, our fonts, etc won't immediately do anything different. To do anything with the upgraded dependencies, the bundle needs to be regenerated, and so devDependencies need to be installed.
If I create a npm and sass-based theme that is derived from sphinx_rtd_theme (assuming the theme is released on npm) I can pull sass includes from the theme. I would need bootstrap to be installed if I were to do that but not necessarily webpack if I want to use my own build environment.
The goal of this would be to allow advanced users to have more advanced customization over the theme. My organization has a framework built on top of bootstrap so, in the future, we could use this to apply our bootstrap theme ontop sphinx_rtd_theme.
@Blendify @agjohnson does this change have any risk of affecting end-users? If we can verify it by the mere fact that our builds keep working, I would see it as low-risk and I think the justification from @Blendify's own organization seems like a good argument :+1:
Keeping this in 1.1 if the above is true, otherwise feel free to push to 2.0 :+1:
The only dependency that really works in this way is jQuery, as we do not bundle jQuery in our artifacts.
All of the other packages are used as source files to produce our artifacts, and so are only devDependencies. Our built bundle works, even if a user is missing the packages Wyrm, Bourbon, FontAwesome, Roboto/etc. All of these packages are bundled into our CSS/JS artifacts.
So, to merge this, I think the only package that should really move to dependencies is jQuery.
Keeping this in 1.1 if the above is true, otherwise feel free to push to 2.0
Seems like a good fit for 2.0