Closed
Bug 1441617
Opened 8 years ago
Closed 8 years ago
Use ES6 imports/exports instead of CJS
Categories
(Tree Management :: Treeherder: Frontend, enhancement, P1)
Tree Management
Treeherder: Frontend
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: emorley, Assigned: emorley)
References
Details
Attachments
(1 file)
We want to switch to ES6 imports since:
* it's now best practice / what contributors will expect to be able to use
* it enables things like tree shaking (dead code removal when building)
I was hoping we could just continue to switch over incrementally as part of other work, however webpack gets angry when mixing and matching ES6 imports with CJS exports - which was causing errors in bug 1441614 and other WIPs I have locally.
As such we should at the least convert the exports we have to ES6, so it doesn't block adding ES6 imports in those files.
| Assignee | ||
Updated•8 years ago
|
Summary: Use ES6 imports/exports for angular modules → Use ES6 imports/exports instead of CJS
Comment 1•8 years ago
|
||
| Assignee | ||
Updated•8 years ago
|
Attachment #8954838 -
Flags: review?(cdawson)
Updated•8 years ago
|
Attachment #8954838 -
Flags: review?(cdawson) → review+
Comment 2•8 years ago
|
||
Commits pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/ce7ad5cc0e8f755ccc393870297c40b698befdb7
Bug 1441617 - Switch the userguide module to ES6 exports
And whilst we're there, use explicit imports instead of relying on
the webpack `ProvidePlugin` to import it for us - which lets us also
remove the eslint global exemption plus unit test workaround.
https://github.com/mozilla/treeherder/commit/6053d8110a1971bcb490ea9805ddbc1ce508257f
Bug 1441617 - Switch the logviewer module to ES6 exports
https://github.com/mozilla/treeherder/commit/588947856af20487fcee2c67d0560a918dda7ab3
Bug 1441617 - Switch the treeherder.app module to ES6 exports
This module is for use by the `index` entrypoint, and inherits from
the shared `treeherder` module.
https://github.com/mozilla/treeherder/commit/5cbad1816280f993a409c7d2872218001b2be202
Bug 1441617 - Remove unused export in perfapp.js
This export is not used, since this file just configures the existing
module (compared to `perf.js` which creates/exports the module).
https://github.com/mozilla/treeherder/commit/7bab404d40af583f1e5573bdcc39259675d43f51
Bug 1441617 - Switch the perf module to ES6 exports
https://github.com/mozilla/treeherder/commit/3e2e22e132828d591575701c03f319b2328b3010
Bug 1441617 - Switch the treeherder module to ES6 exports
This is the shared module that is a dependency of the `treeherder.app`,
`perf` and `logviewer` modules.
https://github.com/mozilla/treeherder/commit/a8e09c5786d15e8f5431bb7bf45b5697fcf1e8ae
Bug 1441617 - Use ES6 imports for angular module dependencies
An AngularJS module declares a dependency on another module like so:
```
const fooModule = angular.module('foo', ['name_of_dep_module']);
```
This is suboptimal, since:
a) the dependency is only validated at runtime not during the build
b) the module name often doesn't match the NPM package name, making
it hard to know whether imports are unused.
As such, many AngularJS packages now export a string containing the
module name, which allows for an ES6-like approach (it still uses
globals underneath, but better than nothing):
```
import nameOfDepModule from 'dep-module';
const fooModule = angular.module('foo', [nameOfDepModule]);
```
Just to complicate things, some modules don't export the module name,
but the actual module object itself. This can't be passed to
`angular.module()` directly, so for these modules we have to use the
`name` property on the module instead:
https://docs.angularjs.org/api/ng/type/angular.Module#name
eg:
```
import depModuleObject from 'awkward-dep-module';
const fooModule = angular.module('foo', [depModuleObject.name]);
```
In fact, the treeherder modules do just that - however its not worth
fixing, since otherwise we'd have to change every file that registers
directives/filters/values/... to fetch the module first, using
`angular.module('name-of-existing-module')`.
https://github.com/mozilla/treeherder/commit/800b31b7193903570e81288247de46e4eb0a1da5
Bug 1441617 - Always import angular explicitly
...rather than relying on `window.angular`.
And also switch to ES6 imports.
https://github.com/mozilla/treeherder/commit/36b930f6fbb9a3ec4f2122f267bea1a73a184250
Bug 1441617 - Move absolute imports above relative ones
Since otherwise once converted to ES6 imports, eslint will report:
`Absolute imports should come before relative imports (import/first)`
See:
https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/first.md
I've split this out of the commit switching to ES6 imports to make
the diff easier to follow.
Note: This changes the order of the CSS imports in userguide and perf,
however I can't see anything that's broken as a result locally.
https://github.com/mozilla/treeherder/commit/ebe97fa18e02705562c40b89aa75b34aec3e463b
Bug 1441617 - Always import mousetrap explicitly
...rather than relying on `window.Mousetrap`.
And also switch to ES6 imports.
https://github.com/mozilla/treeherder/commit/a7df5cd6a079e964905fde4b64d140d4bc0c3dd7
Bug 1441617 - Remove duplicate require() from perf entrypoint
The filters file is already `require()`ed higher up in the file.
This avoids an eslint error once these are switched to ES6 imports.
https://github.com/mozilla/treeherder/commit/b3d02f3874175d7be3d27608408fe665764d70d3
Bug 1441617 - Convert CJS imports to ES6
This converts virtually all of the CJS imports, apart from:
* the partials/plugin template loading in cache-loader - since that
is being handled in bug 1441614
* `.eslintrc.js` / `neutrino-custom/*` - since they are used by node
directly (which doesn't yet support ES6 modules) rather than via
webpack.
File extensions have been omitted for JS/JSX, otherwise eslint reports:
`Unexpected use of file extension "js" for "..." (import/extensions)`
| Assignee | ||
Updated•8 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•