Since it's more reliable (and strict) at code formatting than ESLint.
We use it via an ESLint plugin, and so disable the style-related AirBnB
preset rules, leaving the AirBnB guide to handle only correctness and
best practices rules.
It's highly encouraged to use an IDE integration or Git commit hook
to run Prettier (or `yarn lint --fix`) automatically. See:
* https://prettier.io/docs/en/editors.html
* https://prettier.io/docs/en/precommit.html
We may consider enabling a git commit hook out of the box (using eg
Husky) in the future, however they have previously been known to
interfere with partial-staging workflows, so would need to test the
fixes they have made for them thoroughly first.
In future PRs we may also want to start formatting JSON/CSS/Markdown
using Prettier too.
Imports must now be grouped like so (with newlines between each):
```
// "external" modules
import _ from 'lodash';
import chalk from 'chalk';
// modules from a "parent" directory
import foo from '../foo';
import qux from '../../foo/qux';
// "sibling" modules from the same or a sibling's directory
import bar from './bar';
import baz from './bar/baz';
```
The `import/order` rule has auto-fix support, so any errors can be
resolved using `yarn lint --fix`.
See:
https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/order.md
Neutrino controls our frontend linting, transpilation, source-maps,
testing, dev-server and optimisation of production builds.
Highlights of the upgrade are:
* Major version updates to the individual tools within (such as webpack,
Babel and ESLint), significantly improving performance, fixing
transpilation/minification correctness bugs, adding support for newer
ECMAScript features, and increasing linter coverage.
* Hot reloading in the dev server now works for all entry-points and not
just the jobs view, shortening the feedback cycle.
* Reduced bundle size due to webpack 4's tree shaking, scope hoisting,
automatic shared/vendor code chunk splitting (no need for the manually
maintained 'vendor' list).
* CSS is now extracted out of JS, which improves performance, reduces
bundle size and prevents the initial white flash of un-styled content.
* Support for dynamic imports/code splitting (needed for bug 1502192).
* Support for Jest via a new Jest preset (unblocks bug 1364045).
* Support for public class field declarations (unblocks bug 1480166).
* Improved source-maps (increases the quality of production exception
trace-backs and fixes several debugger breakpoint bugs).
* Reduced amount of custom configuration required for our fairly complex
frontend needs, reducing maintenance burden and allowing for easier
future Neutrino upgrades.
In addition this PR:
* Fixes the WhiteNoise `immutable_file_test()` regex, so that it now
correctly enables browser caching of images, fonts and source maps.
* Enables webpack-dev-server's overlay feature, which displays any
compilation errors in the browser, saving having to switch back
to the console (this can be enabled for warnings too if desired).
* Enables webpack-dev-server's automatic browser-opening feature,
which saves having to manually navigate to `localhost:5000` after
running `yarn start`.
* Switches Karma tests to run Firefox in headless mode, reducing the
workflow disruption when running `yarn test`.
* Uses the new webpack `performance` option to enable maximum asset
file size thresholds, to help prevent bundle-size regressions.
* Rewrites the `package.json` script commands so that they now work
correctly on Windows, even when setting environment variables.
Performance comparison:
* Local `yarn build`:
- Cached: 2m34s -> 23s
- Uncached: 2m34s -> 58s
* Local `yarn start`:
- Cached: 34.5s -> 13.6s
- Uncached: 34.5s -> 31.3s
* Local `yarn test`
- Cached: 61.5s -> 19.8s
- Uncached: 61.5s -> 22.0s
* Local `yarn lint`
- Cached: 3.8s -> 1.8s
- Uncached: 13.7s -> 13.4s
* Travis end-to-end time:
9 minutes -> 6 minutes
* Heroku deploy end-to-end time:
14 minutes -> 9 minutes
This pre-emptively fixes the issues found by the newer ESLint and
ESLint plugins that come with Neutrino 9 - in order to reduce the
size of the Neutrino 9 PR.
Previously it was not possible to test features that required an
authenticated user when:
* using `yarn start` with Vagrant (bug 1363722), which meant slower
watch builds
* pointing the UI at the prod/stage API (bug 1317752), which was
extremely limiting
Now login works in all environments, since the frontend no longer uses a
URL prefix, but instead webpack-dev-server proxies non-webpack URLs to
the chosen `BACKEND_DOMAIN` - avoiding cross-domain issues. Cookies are
rewritten to remove any `secure` directive (which is set on production),
so that they can still be read from HTTP localhost. The `Referer` has to
also be changed to stop Django's CSRF checks from rejecting request.
The slower "build into `dist` and watch" mode is therefore no longer
necessary, so `yarn start:local` instead invokes webpack-dev-server just
like `yarn start` - and the `local-watch.js` workaround has been
removed.
Support for the "publish to GitHub with hardcoded `SERVICE_DOMAIN`"
workflow has been dropped, since it was already rarely used and there is
no way to make it support login.
The API domain environment variable was renamed to `BACKEND_DOMAIN` to
avoid potential confusion given it no longer behaves the same as
`SERVICE_DOMAIN` used to.
NB: For full stack Vagrant workflows users must now connect to port
*5000* on localhost, not 8000.
* enable react/prop-types eslint rule
While enabling this, I set all the PropTypes as ``isRequired`` then ran the app
and wherever I hit issues where the value was undefined, then I removed ``isRequired``.
* Enable react/require-default-props eslint rule
In some cases "null" was the best default. I think this is OK. But having empty arrays
for others meant less checking in the actual code, so bonus! :)
After looking at these, I think I want to go back to using "th" prefix and "ph" prefix.
It is a nice way to see right off the bat that these are constants that are established
by our code and not some external library. So converting ``platformMap`` back to
``thPlatformMap``. It also makes for less code/blame churn as we move to imported
constants.
* Move thOptionOrder to constants.js
* Move thFailureResults to constants.js
* Move thRepoGroupOrder to constants.js
* Move thFavIcons to constants.js
* Move phCompareDefaultOriginalRepo to constants.js
* Move phCompareDefaultNewRepo to constants.js
* Move thDefaultRepo to constants.js
* Move thTitleSuffixLimit to constants.js
* Move phTimeRanges to constants.js
* Move thDateFormat to constants.js
* Move phDefaultTimeRangeValue to constants.js
* Move phTimeRangeValues to constants.js
* Move phBlockers to constants.js
* Move phDefaultFramework to constants.js
* Move phAlertSummaryStatusMap to constants.js
* Move phAlertSummaryIssueTrackersMap to constants.js
* Move phAlertStatusMap to constants.js
* Move thJobNavSelectors to constants.js
* Move thPerformanceBranches to constants.js
* Move phDashboardValues to dashboard.js
* Move phCompareBaseLineDefaultTimeRange to constants.js
* Move thPinboardCountError to constants.js
* Remove now-unused values.js
* Move thResultStatusFilters to constants.js
* Move thEvents to constants.js
* Move thAggregateIds to a aggregateIdHelper.js
* Move thReftestStatus to jobHelper.js
* Remove now-unused provider.js
* Don't use SERVICE_DOMAIN for references to the front-end code
* Change getRootUrl to getApiUrl to better represent what it is for
* Convert other usages of SERVICE_DOMAIN to getApiUrl
* Convert usages of SERVICE_DOMAIN to getProjectUrl
* Convert uses of SERVICE_DOMAIN to getServiceUrl
* Convert tests to use ``urlHelper``
https://eslint.org/docs/rules/no-use-before-define
Three files have been excluded from this rule entirely, since they
would require significant changes that might as well wait until they
are rewritten in React. It's likely these files may also need to be
excluded from future `var` to `let`/`const` conversions as a result.
From the AirBnb style guide GitHub issue comments:
> Some enjoy using hoisting to define helper methods at the bottom of
> the file. This guide discourages relying on hoisting; instead
> suggesting importing the helpers from another file when possible.
This helps prevent:
https://www.owasp.org/index.php/Reverse_Tabnabbing
We're not also using `noreferrer`, since most browsers now support
`noopener` (https://caniuse.com/#search=noopener) and the link targets
are all Mozilla properties where the referrer may be useful.
The auth.js `window.open()` has not been changed, since the login
callback makes use of `window.opener`.
Cameron Dawson
Armen Zambrano G
dependabot-preview[bot]
Sarah Clements
Ed Morley