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)

enhancement

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.
Summary: Use ES6 imports/exports for angular modules → Use ES6 imports/exports instead of CJS
Attachment #8954838 - Flags: review?(cdawson)
Attachment #8954838 - Flags: review?(cdawson) → review+
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)`
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.

Attachment

General

Created:
Updated:
Size: