report: isolate flow-report type checking #12952
Conversation
|
|
||
| const flowResult = JSON.parse( | ||
| fs.readFileSync( | ||
| `${LH_ROOT}/lighthouse-core/test/fixtures/fraggle-rock/reports/sample-lhrs.json`, | ||
| `${__dirname}/../../lighthouse-core/test/fixtures/fraggle-rock/reports/sample-lhrs.json`, |
There was a problem hiding this comment.
similar to #12940 (comment), root.js isn't accessible without importing all of core, so this works for now.
I also don't have a good mental model for where tsx files live on the commonjs <-> esmodules spectrum and what works in them and what doesn't. Does it just depend on the bundler?
| @@ -1,14 +1,37 @@ | |||
| { | |||
| "extends": "../tsconfig", | |||
There was a problem hiding this comment.
I'll switch this back to extending and move most of this back to a tsconfig-base.json or whatever in a follow up
flow-report/App.tsx
Outdated
| @@ -6,8 +6,10 @@ | |||
|
|
|||
| import {FunctionComponent} from 'preact'; | |||
| import {useState} from 'preact/hooks'; | |||
| import type FlowResult from '../types/lhr/flow'; | |||
| import type LHResult from '../types/lhr/lhr'; | |||
There was a problem hiding this comment.
went with explicit type import because this is tsx. Could add declare LH {} to flow-report.d.ts instead if global types are preferred.
There was a problem hiding this comment.
Global types SGTM, I think it would also prevent a merge conflict with #12929
There was a problem hiding this comment.
Global types SGTM
done
flow-report/App.tsx
Outdated
| @@ -6,8 +6,10 @@ | |||
|
|
|||
| import {FunctionComponent} from 'preact'; | |||
| import {useState} from 'preact/hooks'; | |||
| import type FlowResult from '../types/lhr/flow'; | |||
| import type LHResult from '../types/lhr/lhr'; | |||
There was a problem hiding this comment.
Global types SGTM, I think it would also prevent a merge conflict with #12929
fairly straightforward, though a few options here to change now or later, depending on your preferences, @adamraine