8000 [rewrite] the entire build and test system by domq · Pull Request #521 · epfl-si/elements · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[rewrite] the entire build and test system #521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 376 commits into from
Jul 26, 2021
Merged

[rewrite] the entire build and test system #521

merged 376 commits into from
Jul 26, 2021

Conversation

domq
Copy link
Member
@domq domq commented Jul 20, 2021
  • Get rid of Gulp and epfl-si/epfl-elements-toolbox-utils (“the toolbox”); replace with Webpack 5
  • Merge epfl-si/epfl-elements-toolbox-reader and rewrite it as a modern React app (that is, remove as much Redux and as many class components and semicolons as possible)
    • No more client-side Twig rendering in an async Redux reducer; this is now all done at compile-time with Webpack
  • Repair the BackstopJS-based test suite and have it work under Travis / Docker for so-called “hermetic“ (i.e. fully repeatable) tests
  • Modernize almost all dependencies, save for selectize.js which would be a(nother) breaking change — As of today, yarn audit reports 1 Low | 2 Moderate | 1 High

Dominique Quatravaux added 25 commits July 21, 2021 16:55
```
yarn add -D svg-spritemap-webpack-plugin
```
- Same file paths and ID namespaces as in the old toolbox
- Set empty prefix for feather-icons.svg (by hook or by crook, see cascornelissen/svg-spritemap-webpack-plugin#168)
- DRY and halve the size
- Can totally run after the DOM is loaded, thus side-stepping race conditions whence document.body doesn't exist yet
- Ensure that all `<div>`s are hidden — We can no longer assume that the toolbox took care of that at build time
- As a belt to go with these suspenders, put the `<div>` at the end of the `<body>` rather than at the beginning
- Use DOM API consistently and conservatively (https://developer.mozilla.org/en-US/docs/Web/API/Element/insertAdjacentElement#browser_compatibility says .insertAdjacentElement is OK; caniuse.com concurs, assuming you updated your Firefox since 2016)
Also makes the bundle JS files a lot more readable.

- Refactor Copy to accept a single file or pattern in the first argument
- Copy Bootstrap source map, resulting in zero sourcemap-related browser warnings
```
yarn add -D node-polyfill-webpack-plugin
```
According to https://stackoverflow.com/a/65556946/435004 (and the
question above it), Webpack no longer auto-substitutes things like
`require("path")` and there is such a construct in
`node_modules/twig/twig.js`.
There is only one occurrence in App, and it mostly does redux
gyrations which seem to be quite happy being done in
`componentDidMount()` in other places of the code already.
As it turns out, the ages-old concept of “relative URL” works just as
well. WHO WOULDA THUNK IT??! (No pun intended)

There used to be quite a bit of passing around of the document's base
URL, so as to make URLs absolute (for XHR-fetchable things such as
twigs and documentation). In fact, since the whole reader is a
single-page Web app with React-style `/#/` URLs, the base URL for
relative links is always the same. Therefore, “naive” Twig relative
URIs (e.g. in imports) just work.

- Un-plumb all `props.navigation.base_url` / `basePath` baton passing
- Remove the corresponding Redux pipework (setters and reducers)
- go with the new hotness, `useBuiltIns: "usage"` in spite of the sunken costs thereof (next bullet point)
- Worked my way through many a stack trace to get a clear picture of the `sourceType: "unambiguous`
business (https://stackoverflow.com/a/52415747/435004)
- As per the recommendation at https://github.com/zloirock/core-js#index, use a minor version for corejs in webpack.config.js
Configure twig-html-loader to template the twigs out of `assets/config/data.json`
Encapsulates both the model and the Webpack black magic that ships the data to the browser
- Turn `App.jsx`, `Single*.jsx` and `Sidebar*.jsx` elements into function elements
- Keep only the `({ navigation })` Redux channel for these; see below for what happens with the others
- Eliminate `atomicActions` since the (synchronous, compile-time) model of `asset-components.js` obviates the need for it
- Eliminate `({ history })` and `({ location })` everywhere; replace with `useHistory` resp. `useLocation` from "react-router-dom"
- Eliminate `({ document })` by looking up its data from `docs/summary.yml` (move that code from `index.html` into its sole call site in SidebarDocs)
- Eliminate now-unused DocDirLegacy element
- Refactor Item.jsx so that it is actually re-useable (move the gunk into SingleStyleguideItem component, private to SingleStyleguide.jsx)
- Pass Element / Variant model objects around, instead of their properties
- Grow an `onClickHyperlink` optional parameter to `<Item>`, which defaults to eating the click (the exception being pages, when we want to navigate between them)
- Replace `component` with `element` wherever that makes sense
- Fold reader/helpers/parentUrl.js into its sole remaining call site
- Delete `Single.jsx` which is not anyone's superclass anymore
- Tell webpack.config.js to treat .html and .md files under docs/ as pure-text payloads
- Rewrite the `<Doc>` elemennt to `require()` them dynamically, in pretty much the same way asset-components.js does for twig's and YAML's
- No more `window.sources`, `window.sourcesOrder` — Now available at compile-time from `reader/asset-components.js`
- Keep the `({ navigation })` Redux channel; delete all the others
- Delete `window.data` — Not sure when this was last useful, and what for

squash! [delete] As much redux (and other cruft) as we can
…onnected-react-router history nanoid immer twig yaml`

All of these were dependencies of one of the things we moved from run-time to Webpack, or removed altogether (Redux etc.)
This prevents crashes / timeouts in `yarn test` — A backstop for backstop.js, if you will.
`backstop.json` is now a mere build byproduct, as opposed to something we used to read from programmatically, write to (in the same program), and put in Git.

- Rename `backstop_config_updater.js` to `make_backstop_json.js`
- Have `backstop.json` live in `build/`
- Hard-code the bits of data that were previously read back from `backstop.json` into `make_backstop_json.js`
- Introduce async functions `writeJSON` (which pretty-prints the JSON output) and `main`
- Eliminate reading `../toolbox.json`
- Ready for Travis
- Portable (i.e. you always get the ugly Linux fonts, even when `yarn test`ing from a Mac)
- S L O W

Implemented by

- Making `yarn test --docker` work i.e. pass that flag to both `make_backstop_json.js` and `backstop test`
- Under `--docker`, set up target hostname and/or Docker command line so that BackstopJS may reach a server that runs on port 3000 outside of Docker
Does:
- Require a decent version of Node (v0.10.48 or something is... a tad old)
- Plan for sufficient resources for Chrome (garris/BackstopJS#603 (comment))
- Set up the show
- Wait for the show to be ready (i.e. busy-wait for port 3000 to serve)
- Run the test suite

Does not:
- Cut the test suite short when `yarn test --docker` returns nonzero exit code (as it most likely will, every single time)

Does not (yet):
- Auto-promote new Backstop state
- Regenerate dist
- Regenerate gh-pages
@domq domq force-pushed the update/backstop branch from e065959 to 6bb94bc Compare July 21, 2021 15:01
Copy link
Member
@ponsfrilus ponsfrilus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me.

@domq domq merged commit 5ec43d5 into dev Jul 26, 2021
@domq domq deleted the update/backstop branch July 26, 2021 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0