-
Notifications
You must be signed in to change notification settings - Fork 12
[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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…x-reader into feature/halloween-sprint
🎃 Halloween sprint
Features/docs
``` 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
ponsfrilus
approved these changes
Jul 23, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
selectize.js
which would be a(nother) breaking change — As of today,yarn audit
reports 1 Low | 2 Moderate | 1 High