Skip to content

core(lighthouse-logger): convert to ES modules#13720

Merged
connorjclark merged 3 commits into
masterfrom
logger-esm
Mar 7, 2022
Merged

core(lighthouse-logger): convert to ES modules#13720
connorjclark merged 3 commits into
masterfrom
logger-esm

Conversation

@connorjclark

@connorjclark connorjclark commented Mar 4, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

ref #12689

This is not a breaking change because only conversions to ESM inside lighthouse-core would be.

The only impact is that the build-bundle script will see this file as ESM during rollup (FYI: when bundling we use the local copy, but via Node we use the npm pacakge). I have no plans to publish a new version of the npm package, but if we did, that shouldn't be a breaking change for LH consumers either if LH used that new ESM version.

@connorjclark connorjclark requested a review from a team as a code owner March 4, 2022 23:01
@connorjclark connorjclark requested review from adamraine and removed request for a team March 4, 2022 23:01
@connorjclark

connorjclark commented Mar 4, 2022

Copy link
Copy Markdown
Collaborator Author

I have no plans to publish a new version of the npm package, but if we did, that shouldn't be a breaking change for LH consumers either if LH used that new ESM version.

This only holds for usage of LH directly.... if a user was importing the logger package themselves then of course the breaking change would be they can't require it anymore. By punting the publish we don't have to ask ourselves what our versioning policy should be there :)

@connorjclark connorjclark mentioned this pull request Mar 4, 2022
Closed
17 tasks
@connorjclark

connorjclark commented Mar 5, 2022

Copy link
Copy Markdown
Collaborator Author

I think this needs to land first #13722

@connorjclark connorjclark changed the title lib(lighthouse-logger): convert to ES modules core(lighthouse-logger): convert to ES modules Mar 7, 2022
@connorjclark connorjclark merged commit 57c7fea into master Mar 7, 2022
@connorjclark connorjclark deleted the logger-esm branch March 7, 2022 23:55
rayjenn
rayjenn reviewed Jun 15, 2022
View reviewed changes
Comment thread lighthouse-logger/index.js
const marky = require('marky');
import process from 'process';
import debug from 'debug';
import * as marky from 'marky';

This comment was marked as spam.

Sign in to view

@dlockhart

dlockhart commented May 16, 2023

Copy link
Copy Markdown

@connorjclark, @adamraine: Now that this was released ~2 hours ago, it's started blowing up. Specifically this usage in chrome-launcher:
https://github.com/GoogleChrome/chrome-launcher/blob/main/src/chrome-finder.ts#L13

@adamraine

adamraine commented May 16, 2023

Copy link
Copy Markdown
Contributor

@dlockhart thanks, yeah we are working on a fix

This was referenced Nov 27, 2023
Open
Open
This was referenced Nov 27, 2023
Open
Open
Open
@MaxMood96 MaxMood96 mentioned this pull request Nov 28, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@rayjenn rayjenn rayjenn left review comments

@adamraine adamraine adamraine approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@adamraine adamraine

Labels

waiting4reviewer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@connorjclark @dlockhart @adamraine @rayjenn @devtools-bot