Skip to content

Conversation

@adamraine
Copy link
Contributor

@adamraine adamraine commented Nov 15, 2022

#14441 (comment)

  • Global util types (SelfMap, Immutable, etc) are moved to util.d.ts and are accessible via LH.Util
  • Global namespace modifications (Intl, Window) are moved to node.d.ts
  • Any remaining external types (Flags, RunnerResult, etc) are staying in externs.d.ts

This is to separate global definitions only useful to us and non-global definitions we eventually want to expose.

@adamraine adamraine requested a review from a team as a code owner November 15, 2022 23:04
@adamraine adamraine requested review from connorjclark and removed request for a team November 15, 2022 23:04
@adamraine adamraine changed the title core: breakup externs.d.ts core: remove globals from externs.d.ts Nov 15, 2022
@brendankenny
Copy link
Contributor

Global util types (SelfMap, Immutable, etc) are moved to util.d.ts and are accessible via LH.Util

These seem like a prime subject for real imports instead of being in a new LH namespace. They're mostly used in d.ts files and they've never been namespaced before so no need to update existing usage. Some of the uses of them aren't needed as well...e.g. LH.Util.Immutable<LH.Config.Settings> should really just be LH.Audit.Context['settings'] in a lot (all?) of the updated cases.

@adamraine
Copy link
Contributor Author

They're mostly used in d.ts files and they've never been namespaced before so no need to update existing usage.

Most of the usages I had to update were in .js files. Also importing these directly with jsdoc requires a template:

/**
 * @template T
 * @typedef {import('../types/util.js').default.SelfMap<T>} SelfMap
 */

e.g. LH.Util.Immutable<LH.Config.Settings> should really just be LH.Audit.Context['settings'] in a lot (all?) of the updated cases.

I'll try and update as many of these as I can.

@@ -0,0 +1,72 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

wdyt about types/utility-types.d.ts? Even if redundant it self describes a little better and differentiates more from the four util.* files we already have

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure but still exposed as LH.Util?

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.

4 participants