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

Expose TypeScript typings #1773

Closed
mohsen1 opened this issue Feb 24, 2017 · 33 comments
Closed

Expose TypeScript typings #1773

mohsen1 opened this issue Feb 24, 2017 · 33 comments

Comments

@mohsen1
Copy link
Contributor

mohsen1 commented Feb 24, 2017

I see some TypeScript files here and there... is this written in TypeScript? Even if it's not, all of JavaScript code is nicely annotated (Thanks Closure Compiler!). Can you expose the typings via typings key in package.json? It would be great for TypeScript users and VSCode editor users thanks to Automatic Type Acquisition

@ebidel
Copy link
Contributor

ebidel commented Feb 24, 2017

Only the CLI is written in TS atm. Everything else is closure-based JS.

Almost no one on the core team has TS experience, so we'd love your help. @samccone also loves TS. Maybe he can help.

@wardpeet
Copy link
Collaborator

wardpeet commented Feb 24, 2017

@ebidel do we want to move to typescript?

@samccone
Copy link
Contributor

The idea was to test run ts w/ the CLI to start, and then reevaluate.
I currently do not work on lighthouse as much as I initially did, so this decision is more up to the maintainers of the lib ;)

@ebidel
Copy link
Contributor

ebidel commented Feb 25, 2017 via email

@wardpeet
Copy link
Collaborator

I agree with @ebidel but might be good to put this on the next lighthouse meeting

@devrelm
Copy link
Contributor

devrelm commented Jun 15, 2017

I'd like to ask that this be revisited. I'm using chrome-launcher in a typescript project, but am getting errors because the d.ts files are not included. It should be a fairly simple change to the tsconfig and package.json to make sure that those are produced and published — I'm more than willing to put in a PR (in fact, I'm in the process of forking & cloning the repo as we speak).

@samccone
Copy link
Contributor

samccone commented Jun 15, 2017

Hey @devrelm so, you can directly require the typescript code for the launcher (no need for d.ts), will this work for you?

yarn add chrome-launcher
import {launch} from 'chrome-launcher/chrome-launcher';

@devrelm
Copy link
Contributor

devrelm commented Jun 15, 2017

@samccone Yeah, that'll work. Another alternative could be renaming chrome-launcher.ts to index.ts, or (to not break backwards compatibility with people already implementing your solution) create a separate index.ts containing:

export * from './chrome-launcher';

Then update "main" in the package.json to point to index.js and everything should work, at least for those who have their tsconfig set up with "moduleResolution": "node".

Anyway, it's not a pressing issue now that I know the workaround.

Thanks!

@paulirish
Copy link
Member

since folks have run into this twice in two days, i think doing the above makes sense. that work for you, @samccone?

@paulirish
Copy link
Member

paulirish commented Jun 16, 2017

@devrelm the #2513 fix is shipped in [email protected]
thx very much for the suggestion!


update mar 2018: I have an early branch up here with additional types: https://github.com/GoogleChrome/lighthouse/blob/typechecking/typings/externs.d.ts

and also https://github.com/GoogleChrome/lighthouse/blob/e12576fe12e48307d60c5e3f0279ebfa57f848f2/lighthouse-core/test/lib/validate-lhr.js

@nicky-lenaers
Copy link

nicky-lenaers commented Sep 11, 2017

@paulirish @wardpeet @ebidel This is great. I can now use the typed interfaces for chrome-launcher. However, I use lighthouse to programmatically test output scores. I came across lighthouse-mocha-example, but that's only available as a Docker container. I'm looking for a more integrated solution for rapid local development. So I implemented a test-pwa-score.ts file (ran with ts-node) that looks a lot like the test file from the official Angular repository. Regarding this section:

const flags = { };

const scores = {
  'pwa': 90,
  'performance': 90,
  'accessibility': 90,
  'best-practices': 90
};

launchChromeAndRunLighthouse('http://localhost:8000', flags, null)
.then((results: any) => { // <-- this line
  results.reportCategories.forEach((cat) => {

    if (scores[cat.id]) {

      if (cat.score < scores[cat.id]) {
        console.log(`Score for '${cat.id}' expected to be ${scores[cat.id]}, but got ${cat.score}`);
      } else {
        console.log(`Score for ${cat.id} OK, ${cat.score} satisfies theshold ${scores[cat.id]}`);
      }
    }

  });
});

The results parameter in the Promise callback contains a huge JSON file, containing quite some levels of objects and arrays. Currently I am forced to type this as any, because I don't have an available interface for this to use with TS. The object is way too big to provide a quick interface myself.

Will this be available anytime soon?

@underbyte
Copy link
Contributor

The CLI isn't TS anymore. Think this one can be close.

@patrickhulce
Copy link
Collaborator

@brendankenny now that we've got everything nicely typed up what do you think the best strategy is?

It seems like there's different levels we could go with this that all have different exposure risk of breaking changes. I'd be inclined to go as small as possible and expose some LHR-lite style types for the response.

@arnold-fingerfood
Copy link

Thanks for bumping it back up, Patrick.
#7089

@brendankenny
Copy link
Member

I'd be inclined to go as small as possible and expose some LHR-lite style types for the response.

Yeah, agreed with this. We can start with the LHR and the callable interface to lighthouse-core/index.js and move on from there as needed (e.g. the primary audit interfaces may be helpful for anyone writing plugins)

@taji
Copy link

taji commented Feb 14, 2019

Sorry, is this fixed in lighthouse 4.1.0? Looking at this project:

https://github.com/AndrejsAbrickis/lighthouse-azure-devops

as described here:

https://medium.com/@andrejsabrickis/continuously-audit-web-apps-performance-using-google-s-lighthouse-and-azure-devops-3e1623372f79

He's using an alpha version of 4.0.0 in his package.json:

"lighthouse": "^4.0.0-alpha.2-3.2.1"

and he's importing as so:

import lighthouse from 'lighthouse/lighthouse-core';

But this isn't working in 4.1.0. I'm thinking these alpha changes weren't rolled into the release?

@patrickhulce
Copy link
Collaborator

@taji that "alpha" version is actually identical to version 3.2.1

In v4, you should be able to achieve the same effect by replacing

/// <reference path="./node_modules/lighthouse/typings/externs.d.ts"/>

with

/// <reference path="./node_modules/lighthouse/types/externs.d.ts"/>

We're hoping to offer a better solution sometime in the future that doesn't require this hacky reference though.

@alekseykulikov
Copy link
Contributor

TS typing would be especially useful for plugins!

I tried hard to make it work in https://github.com/treosh/lighthouse-plugin-field-performance
But it's not possible without "noImplicitAny": false which breaks the purpose of types.

@ali-habibzadeh
Copy link

  1. Any updates?

@mb-jenks
Copy link

mb-jenks commented Mar 5, 2020

Also interested in this. Any way to export the current types in the repo or put into an @types package?

@wardpeet
Copy link
Collaborator

wardpeet commented Mar 5, 2020

Moving to types repo gives us extra maintenance overhead so I think we don't want to go that route but exposing it from our package seems a good idea

@mohsen1
Copy link
Contributor Author

mohsen1 commented Mar 5, 2020

if your types are not generated from code, I suggest keep the types in DefiantlyTyped. Almost every lib that decided to publish handwritten types has regretted it.

@connorjclark
Copy link
Collaborator

I don't think we were ever going to write types manually. We were waiting for TS 3.7 to generate types from JSDoc.

@brendankenny should we move on this now?

@brendankenny
Copy link
Member

I don't think we were ever going to write types manually. We were waiting for TS 3.7 to generate types from JSDoc.

@brendankenny should we move on this now?

it was still buggy the last time I tried it, but maybe it works now? We have a large amount of stuff we would probably not want to have in end-user types, but maybe there's also a nice way to do that.

We also need to move off types in a global namespace (we need to do that for a few reasons, actually), but I wasn't sure how well that would go over with y'all :)

@connorjclark
Copy link
Collaborator

We also need to move off types in a global namespace (we need to do that for a few reasons, actually), but I wasn't sure how well that would go over with y'all :)

What would that look like? For example, to reference LH.Artifacts, would we require a module artifacts.js, or lh.js which would export Artifacts? I'm fine with either, btw.

Sounds like we are a few steps removed from doing this properly. For now, let's just upgrade to the latest TS.

@phenomnomnominal
Copy link

Would love to consume these types, is there anything I can do to help move this along?

@ali-habibzadeh
Copy link

Any updates?

@ali-habibzadeh
Copy link

ali-habibzadeh commented Apr 26, 2021

Anything?

@kootoopas
Copy link

Since there has been no response for quite a while, would it be a reasonable suggestion to programmatically edit the contents of lighthouse's package.json in order for the typings section to be added via the postinstall hook?

@pan-alexey
Copy link

same issues

@Pagan-Idel

This comment was marked as spam.

@rdadoune
Copy link

rdadoune commented Jul 9, 2022

Create a lighthouse.d.ts file and add this to it:

/// <reference types="lighthouse/types/global-lh" />

declare module 'lighthouse' {
  function lighthouse(
    url: string,
    options: Partial<LH.CliFlags>,
  ): Promise<LH.RunnerResult>;
  export = lighthouse;
}

Then in your tsconfig.json file add lighthouse.d.ts to the include option. VSCode and tsc will pick it up automatically from there. If you also use ts-node, then you will also need to add lighthouse.d.ts to your files array, it will complain if you don't, I'm not sure why.

{
  "compilerOptions": {
    /* ... */
  },
  "files": ["lighthouse.d.ts", /* ... */],
  "include": ["lighthouse.d.ts", /* ... */]
}

@adamraine
Copy link
Member

The types should be exposed in Lighthouse 10.0 with #14441

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

No branches or pull requests