use eject from create React app to allow more robust dev and production build configs

[christopher-johnson]

the current webpack config is minimal. An ejected configuration from create react app provides reasonable defaults for custom configurations which will provide both development and production build environments.

For example, the standard dev configuration includes helpful plugins like babel-plugin-transform-react-jsx-source that provides stack traces with line numbers and file names.

It has been indicated in the meeting today that this repo is considered to be a starting point for future development. Is a PR for this welcomed here?

[mejackreed]

A PR is welcomed sure, to outline the changes. It may be a good point for discussion of the approach and its tradeoffs.

An earlier concern I had (although might have been incorrect) is that CRA focuses mainly on projects that are building single page web applications, not libraries. Going off the README here: https://github.com/facebook/create-react-app#popular-alternatives

Here are few common cases where you might want to try something else:
... if you’re not building a single-page app
... If you need to publish a React component

While I think Mirador hits many of these checkboxes, I wouldn't consider it adhering to a traditional single page application. Another reason for the "start from scratch approach" was to make sure that we had broad understanding of the various parts of our webpack config and project setup.

All that being said, enhancements to the project structure are most welcome! I just wanted to provide a bit of context of how we got here and some of the original concerns with CRA.

[christopher-johnson]

this issue and PR is linked to the history branch which is now 8 commits ahead of master. As such, it probably cannot be merged as is. I can cherry pick any commit (or whatever) into another branch and PR at your discretion. Just let me know how / if we can move forward and if I should close this PR. If it helps, I can create some documentation / diagrams to illustrate how I think that this structure will work as the code base grows.

There are a couple of issues beyond the somewhat radical restructuring that I believe are good practices, but are opinionated. For example, using the .jsx extension for React Components. I also think that named exports are preferable to default in pure ES6. (ref. "tree shaking").

With WindowTopBar as a module, I had to add mapStateToProps to get the windowIds. Also, it seems to require store as a prop. Not sure why it works in your build without that.

The e2e test package can now build from imported apps. The CI may seem a bit complex, but this is due to the monorepo. e2e tests are usually run against published packages and not against build artifacts, but building lib apps from the monorepo on demand is an option. Cypress has a video record feature that is useful.

For reference, here is the latest build log.

[mejackreed]

Ok, so looking at this and based off of our conversations in Edinburgh, I see the smoothest way forward to get these pieces in, and also provide shared understanding to the entire team as this:

1. Split out necessary Webpack config changes separately into a PR. While the approach in splits core and app into separate packages, adds complete configuration for dev and production  #69 adds a bunch of great functionality, it also now sets us up with a 1000 lines of Webpack config code to maintain. I'm concerned about having shared understanding and ownership over this config. Could we instead do the following:

• Implement proposed babel-plugin-transform-react-jsx-source as a separate PR if possible
• Implement a minimal production config as a separate PR if possible
• Implement any additional webpack niceties as separate PRs if possible

As I briefly mentioned, I several common things that we would do in modern front end applications may not apply in this repo (chunking, service worker, etc) but we don't want to preclude Mirador adopters from doing any of this.

2. Separate a PR that moves our test suite to cypress. Is this possible without yet implementing mono-repo construct? I'm a little hesitant to start splitting out packages at this point, until we have better shared understanding about this.

3. I'd be 👍 to moving components to the jsx file extensions. But I think there was significant discussion about this previously. Perhaps a separate PR that does this would allow that discussion to take place on it? Does this have any impact on plugin authors?

4. I think we need to have the monorepo discussion also, perhaps that is worth having at some point in January when we are face to face. It may not be a problem we have to solve just yet so I would hesitate on moving that direction until we are all on the same page.

5. Are there other ES6 / treeshaking /module approaches you would like to propose? I think generally those small improvements are most welcome.

6. Are there other large pieces of splits core and app into separate packages, adds complete configuration for dev and production  #69 to consider? I'm 👍 for improving developer experience here, just in a piece by piece way if possible.

Thanks for all of your hard work on this @christopher-johnson . I'm also happy to hop on a pairing session to see if we can knock one of these out together.

[christopher-johnson]

I see the path forward and will definitely work with you on this. I would start by adding the webpack build to create the lib package as this is a necessity for people who want to import a basic app from npm.

btw, today I have integrated the proof of concept into a React CMS ( an spa that uses react-router) to test whether it was possible. This manuscript portal project needs a CMS and I prefer to use React and ES6 imports. It actually works. You can check it out here:

This was actually not so easy as there is an issue with react-redux 6.0.0 and the mirador store when passed down through the router (the app components could not find the context). Oddly, it works when using react-redux 5.1.1 (I do not know why yet).

This kind of external integration also highlighted the question of inline vs. bundled styling. Not sure what the optimal solution is, but the bundled default styles do not work in an external container.

[mejackreed]

Yep looks like react-redux introduces a breaking API change: https://github.com/reduxjs/react-redux/releases/tag/v6.0.0

Passing store as a prop to a connected component is no longer supported. Instead, you may pass a custom context={MyContext} prop to both and . You may also pass {context : MyContext} as an option to connect.

Mirador already has an npm package which we will release as part of the package once we port the POC code into the main repo. We will of course make sure to release on npm in the appropriate ways. Installation via NPM is one of the main targets we want to hit.

[christopher-johnson]

1. added updates to webpack4 #77 to update webpack and loaders to current versions.
2. see add build script for main npm package with named module export #78 add a build script to produce an importable package library. This is different than the distribution package, since it would not include <Provider>, store or ReactDOM.render as these would be implemented by the consumer importing <App> into its own index.js

After 2, then I guess implementing cypress and improving the test tooling would be next. There is an issue #72 to add code coverage, which can be done by adding this config here

I think that moving to cypress does not require the monorepo. It should work in any case. The question is how much flexibility it would need. I believe that it should support multiple apps and have its own build scripts to create them, but this can be added incrementally to a simple start.