convert to ES modules #354
Conversation
connorjclark
force-pushed
the
esm
branch
2 times, most recently
from
d719ca8 to
72de11d
Compare
|
re: failures
|
|
Confirmed working with GoogleChrome/lighthouse#14241 |
| @@ -70,7 +71,7 @@ const HEADINGS = [ | |||
| class AdBlockingTasks extends Audit { | |||
| /** | |||
| * @return {LH.Audit.Meta} | |||
| * @override | |||
| * /override This member cannot have a JSDoc comment with an '@override' tag because its containing class ... does not extend another class. | |||
There was a problem hiding this comment.
Can we remove these comments?
There was a problem hiding this comment.
Yeah, I'll remove the entire line.
There was a problem hiding this comment.
BTW I'm not sure why this error occurs now, but it didn't seem worth the effort to figure it out.
| @@ -1,3 +1,5 @@ | |||
| // LHCI currently does not support .cjs, so generate a json config with: | |||
There was a problem hiding this comment.
Any reason for keeping this file if unsupported? Any issue with just keeping the json file?
There was a problem hiding this comment.
just that .cjs is easier to author, and it is on my backlog to update LHCI to support ESM config/at least support .cjs and then I'll come back and delete the JSON here.
I can do as you suggest if you'd prefer.
package.json
Outdated
| @@ -21,13 +22,15 @@ | |||
| "dependencies": { | |||
| "@tusbar/cache-control": "^0.3.1", | |||
| "@types/esprima": "^4.0.2", | |||
| "lighthouse": "^8.6.0", | |||
| "lighthouse": "next", | |||
There was a problem hiding this comment.
I worry that if there are breaking changes they will no longer be version tied, and older versions will fail to work. Does this seem like a reasonable concern?
There was a problem hiding this comment.
I'll change this to a specific nightly version. And modify again when 10.0 is actually released, ~Sept.
If you'd rather not cut an actual release to npm, that'd be fine. We can always tie Lighthouse's version of pubads to a branch or specific commit during the transition.
@connorjclark The smoke test seems to be flaky, trying to see if rerunning works. But I've gone ahead and temporarily remove LHCI from the required actions list. |
|
LGTM, I think best to wait for a stable LH dependency before cutting a new prod release. |
Lighthouse 10.0 will be ES modules (#12689) which will require plugins to be ES modules too. Here's a PR that updates everything in this repo to ES modules.