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: convert core d.ts files to modules #12880

Merged
merged 10 commits into from
Aug 9, 2021
Merged

core: convert core d.ts files to modules #12880

merged 10 commits into from
Aug 9, 2021

Conversation

brendankenny
Copy link
Member

@brendankenny brendankenny commented Aug 7, 2021

re: #12860

in the style of #12870, converts all the rest of our core .d.ts files to local import/export style with a single file for exposing on the global. No changes to js files here. externs.d.ts is the only remaining (core) file, left for a follow up because it's messier.

No changes to .js files in this one. ?w=1 to avoid having to wade through all the indentation changes, and the individual file conversions (and their side effects) are split up by commit.

@brendankenny brendankenny requested a review from a team as a code owner August 7, 2021 01:03
@brendankenny brendankenny requested review from connorjclark and removed request for a team August 7, 2021 01:03
@google-cla google-cla bot added the cla: yes label Aug 7, 2021
Copy link
Member Author

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@connorjclark sorry for the slog of a PR :)

/** Errors preventing page being installable as PWA. This moved to a regular artifact in Fraggle Rock. */
InstallabilityErrors: Artifacts.InstallabilityErrors;
/** A set of page-load traces, keyed by passName. */
traces: {[passName: string]: LH.Trace};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these remaining LH.* references are all in externd.d.ts, which is a real diverse grab bag of types :)

}
// Result namespace
declare module Result {
interface Environment {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this moves LH.Environment to LH.Result.Environment, but the only reference to this type by name is in this file (on LH.Result['environment']), so it doesn't really matter. It just felt weird to be out on top with all the other subtypes already nested under LH.Result.

artifacts?: Partial<Record<keyof LH.Artifacts|'_maxChromiumMilestone'|'_minChromiumMilestone', any>>
networkRequests?: {length: number};
}
declare global {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is equivalent to before (Smokehouse is a global type), but as soon the import lines were added above the file became a module, so it needed an explicit declare global

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.

3 participants