Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Mar 7, 2022
Merged

Conversation

connorjclark
Copy link
Collaborator

@connorjclark connorjclark commented Mar 4, 2022

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
Copy link
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
17 tasks
@connorjclark
Copy link
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
const marky = require('marky');
import process from 'process';
import debug from 'debug';
import * as marky from 'marky';

This comment was marked as spam.

@dlockhart
Copy link

@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
Copy link
Member

@dlockhart thanks, yeah we are working on a fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants