Merge dash renderer with DashPy

[HammadTheOne]

This PR addresses plotly/dash-core#225 as part of the Dash monorepo project, and merges the dash-renderer artifacts within Dash. Dependencies and resources are loaded from Dash itself, which will make the release and development process a bit easier. This PR also introduces gulpfile to the Dash repo as a tool for moving and modifying files/directories.

Contributor Checklist

• I have broken down my PR scope into the following TODO tasks

  • Merge dash-renderer artifacts
  • DashPy init.py will need to be aware of the dynamically defined resources
  • DashPy needs to get renderer resources from itself instead of dash_renderer
  • Set up gulpfile to automate renderer updates merge to dash
  • Fix following glob sequence in MANIFEST: include dash/dash-renderer/**/**/**/**/** (Fixed in 9900303)
  • Add cleanup job for node_modules (Fixed in 317d7e8)
  • Change build folder to current_directory .

• I have run the tests locally and they passed. (refer to testing section in contributing)

• I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

• I have added entry in the CHANGELOG.md
• If this PR needs a follow-up in dash docs, community thread, I have mentioned the relevant URLS as follows
  • this GitHub #PR number updates the dash docs
  • here is the show and tell thread in Plotly Dash community

Closes plotly/dash-core#225

[alexcjohnson]

@HammadTheOne the final renderer build in dash/deps looks great, but I wonder about the process - can't we just use the same process we already had - renderer build that calls out to build_process, as well as the webpack output - and point it at dash/deps instead of dash-renderer/dash_renderer?

My rationale here is that as you have it here we're duplicating files: the ones actually getting used by dash are going to be harder to keep updated during renderer development because they need to go through an extra step, and the ones being created inside the renderer directory never get directly used at all. Also dash_renderer/__init__.py (based on templating from init.template) is a little cleaner than the new _dash_renderer.py, as it has all the versions inlined.

Finally (and again this is thinking about development in the renderer itself) since the entire dash/deps directory is build artifacts based on source files elsewhere in the repo, it should be gitignored except in the released (master) branch.

[HammadTheOne]

@HammadTheOne the final renderer build in dash/deps looks great, but I wonder about the process - can't we just use the same process we already had - renderer build that calls out to build_process, as well as the webpack output - and point it at dash/deps instead of dash-renderer/dash_renderer?

I think that makes a lot of sense. I've removed the gulpfile method entirely, and modified the existing build process to make it a little more flexible and to update and source dependencies directly to and from the deps directory. A couple of things to note - in the existing build_process call relied on the dash-renderer package being installed and present in the environment. Since that package will no longer exist, I've moved the dash-renderer directory into the dash package so it's added to the source dist and the existing scripts work in line with it.

My rationale here is that as you have it here we're duplicating files: the ones actually getting used by dash are going to be harder to keep updated during renderer development because they need to go through an extra step, and the ones being created inside the renderer directory never get directly used at all. Also dash_renderer/init.py (based on templating from init.template) is a little cleaner than the new _dash_renderer.py, as it has all the versions inlined.

Updated in 9f8e0aa, with slight modifications to the existing template to have the correct relative url.

Finally (and again this is thinking about development in the renderer itself) since the entire dash/deps directory is build artifacts based on source files elsewhere in the repo, it should be gitignored except in the released (master) branch.

I'm a little confused on this point - presently, we can gitignore the dependencies in the existing dash_renderer directory because they'll be sourced from the dash-renderer package which is installed alongside dash. However, when that it will no longer exist, I think we might need to leave these in the deps directory in order to directly source them from within dash, or the dev branch version will fail to import it.

[alexcjohnson]

I think we might need to leave these in the deps directory in order to directly source them from within dash, or the dev branch version will fail to import it.

That's right, the committed code in dev branches will not run on its own. In the dev workflow, it's normal that after checking out the branch you want, you need to run the build command to create the relevant bundles & artifacts before you can actually run dash.

[HammadTheOne]

@alexcjohnson I've made some changes to the CI config so that the artifacts are built within the dash package before installing and running tests and those are passing now.

Other than that, I think this PR might be ready for a final pass, if the dependency structure makes sense at this point.

[jonmmease]

@alexcjohnson @HammadTheOne Is there anything else we need to pull the trigger on this one?

[alexcjohnson]

This will include all the source files, won't it? I don't think we want to do that. The old dash-renderer MANIFEST.in is just:

include package.json
include digest.json
include dash_renderer/*.js
include dash_renderer/*.map

In the new structure, dash_renderer/*.js is replaced by dash/deps/*.js which you have above. I'm not aware of any value that package.json or digest.json have to end users so we should be able to ignore those...

Then there's the question of the *.map files: you're not currently including them, and in fact in the past in response to a user request to reduce the package size #810 we at one point removed them #910 but then for some reason that I can't find any record of we immediately reverted that 369ace9 before making a release. @Marc-Andre-Rivet do you have any memory of that? I don't see a PR or any discussion, just a revert commit

Anyway, is there anything else we really do want from the dash-renderer directory, or can we drop this line (as well as the line below that only exists in reference to this one)?

[HammadTheOne]

Updated in afc363c. We can safely drop the other lines

I'm noticing that .map files aren't being generated by the webpack for souce-mapping. Is this because we have inline-source-map?

[alexcjohnson]

Ah hmm, actually this changed in #1576 in a way that I hadn't recognized - @Marc-Andre-Rivet please confirm I'm understanding this correctly: the ONLY case we're generating a sourcemap now is when you run webpack-serve (which is what you get with npm start). The other build commands now seem to be all the same (as far as the bundle itself is concerned) and create no sourcemap. npm run build:js and build:dev are now literally the same command.

Unfortunately I can't actually run npm start ATM, and I don't think it does what we want anyway - really I'd like a command that outputs the same files we currently create, but stays open watching the source files for changes. @Marc-Andre-Rivet didn't we have that in the renderer at some point? Am I missing something about the updated tooling?

[alexcjohnson]

Excellent! 💃 once there's an issue open to follow up on sourcmaps #1611 (comment)