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

misc: add module types declaration #13523

Closed
wants to merge 2 commits into from

Conversation

Dumeng
Copy link

@Dumeng Dumeng commented Dec 22, 2021

Summary

Feature: Add module declaration file for better typescript support.
The types is generated using below command.
npx /lighthouse-core/index.js --declaration --allowJs --emitDeclarationOnly --outDir types

For current version, if we create a TS project and attempt to import lighthouse, it will throw an error about missing module types declararion.

TS doc for package.json types field
TS doc for generating .d.ts files from .js files

Related Issues/PRs

#1773

@Dumeng Dumeng requested a review from a team as a code owner December 22, 2021 07:02
@Dumeng Dumeng requested review from adamraine and removed request for a team December 22, 2021 07:02
@Dumeng Dumeng changed the title misc: Add module types declaration misc: add module types declaration Dec 22, 2021
@brendankenny
Copy link
Member

Have you tried using these types? I don't believe they'll work because they reference global lighthouse types (under the global.LH namespace) that won't be defined for users of this file.

For #1773 to really be solved, I think the exported types will need to be written manually, the global LH types will have to eliminated, or some fancy api-extractor magic will need to happen (or some combo of the three).

@Dumeng
Copy link
Author

Dumeng commented Dec 23, 2021

You are right. I refer the global types in my test file, so it looked fine for me.

I have commit an updated version.

Because the project already has type files for most of source code. So this PR only generated the types for the main index.js. I think it's a good start. And now I added manually type declarations to the modified the auto-generated file. Maybe it's not the final version, but I hope it will cover the most common scenario.

@connorjclark
Copy link
Collaborator

We can't accept a solution that isn't 100% done and automated, and we can't prioritize working on this at the moment.

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.

5 participants