lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Expose TypeScript typings

Open mohsen1 opened this issue 8 years ago • 32 comments
trafficstars

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

mohsen1 avatar Feb 24 '17 02:02 mohsen1

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.

ebidel avatar Feb 24 '17 04:02 ebidel

@ebidel do we want to move to typescript?

wardpeet avatar Feb 24 '17 19:02 wardpeet

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 ;)

samccone avatar Feb 24 '17 22:02 samccone

Right. I don't think we want to move anything else over given the state of the team. There doesn't seem to be much benefit in moving more to TS atm.

On Fri, Feb 24, 2017, 12:39 PM Sam Saccone [email protected] wrote:

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 ;)

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/GoogleChrome/lighthouse/issues/1773#issuecomment-282425205, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOigD-6qHKptXTtzjXhlAJqPR8VaOiVks5rf1wtgaJpZM4MKwBk .

ebidel avatar Feb 25 '17 03:02 ebidel

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

wardpeet avatar Feb 26 '17 19:02 wardpeet

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).

devrelm avatar Jun 15 '17 12:06 devrelm

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';

samccone avatar Jun 15 '17 14:06 samccone

@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!

devrelm avatar Jun 15 '17 22:06 devrelm

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

paulirish avatar Jun 16 '17 01:06 paulirish

@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

paulirish avatar Jun 16 '17 16:06 paulirish

@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?

nicky-lenaers avatar Sep 11 '17 11:09 nicky-lenaers

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

underbyte avatar May 02 '18 03:05 underbyte

@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.

patrickhulce avatar Jan 24 '19 21:01 patrickhulce

Thanks for bumping it back up, Patrick. https://github.com/GoogleChrome/lighthouse/issues/7089

arnold-fingerfood avatar Jan 24 '19 22:01 arnold-fingerfood

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)

brendankenny avatar Jan 28 '19 21:01 brendankenny

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?

taji avatar Feb 14 '19 13:02 taji

@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.

patrickhulce avatar Feb 14 '19 14:02 patrickhulce

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.

alekseykulikov avatar May 31 '19 13:05 alekseykulikov

  1. Any updates?

ali-habibzadeh avatar Jan 31 '20 12:01 ali-habibzadeh

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

mb-jenks avatar Mar 05 '20 15:03 mb-jenks

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

wardpeet avatar Mar 05 '20 17:03 wardpeet

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.

mohsen1 avatar Mar 05 '20 18:03 mohsen1

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?

connorjclark avatar Mar 05 '20 19:03 connorjclark

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 :)

brendankenny avatar Mar 13 '20 20:03 brendankenny

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.

connorjclark avatar Mar 13 '20 22:03 connorjclark

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

phenomnomnominal avatar May 15 '20 14:05 phenomnomnominal

Any updates?

ali-habibzadeh avatar May 30 '20 11:05 ali-habibzadeh

Anything?

ali-habibzadeh avatar Apr 26 '21 18:04 ali-habibzadeh

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?

kootoopas avatar Aug 26 '21 20:08 kootoopas

same issues

pan-alexey avatar Aug 30 '21 08:08 pan-alexey