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

core(tsc): remove more reliance on implicit index signatures #5874

Merged
merged 2 commits into from
Aug 21, 2018

Conversation

brendankenny
Copy link
Member

part of #5873

No real theme here, just scattered fixes where none needed major interventions. Resolves 23 errors when using typescript@next.

The other side of some of these fixes is going explicitly any or ts-ignore, but it didn't seem worth losing refactoring, intellisense, etc for most of these

@@ -73,9 +76,11 @@ class NetworkAnalyzer {
return summaryByKey;
}

/** @typedef {{record: LH.Artifacts.NetworkRequest, timing: LH.Crdp.Network.ResourceTiming, connectionReused?: boolean}} RequestInfo */
Copy link
Member Author

Choose a reason for hiding this comment

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

I apologize for the terrible name here, @patrickhulce :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

heh, works for me :)

@@ -368,6 +368,7 @@ class Runner {
const ArtifactClass = require('./gather/computed/' + filename);
const artifact = new ArtifactClass(computedArtifacts);
// define the request* function that will be exposed on `artifacts`
// @ts-ignore - doesn't have an index signature, so can't be set dynamically.
computedArtifacts['request' + artifact.name] = artifact.request.bind(artifact);
Copy link
Member Author

Choose a reason for hiding this comment

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

artifact and ArtifactClass are already any here, and computedArtifacts already has a TODO to refactor above this method, so it seems fine to just ignore and keep the issues contained here than to make computedArtifacts have a generic index signature and have it leak into audits, etc.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

cool cool cool

@brendankenny brendankenny changed the title core(tsc): remove more reliance on implicit index signatures core: remove more reliance on implicit index signatures Aug 21, 2018
@brendankenny brendankenny changed the title core: remove more reliance on implicit index signatures core(tracing-processor): throw on no toplevel events Aug 21, 2018
@brendankenny brendankenny changed the title core(tracing-processor): throw on no toplevel events core(tsc): remove more reliance on implicit index signatures Aug 21, 2018
@brendankenny brendankenny changed the title core(tsc): remove more reliance on implicit index signatures core(tsc): remove more reliance on implicit index signature Aug 21, 2018
@brendankenny brendankenny changed the title core(tsc): remove more reliance on implicit index signature core(tsc): remove more reliance on implicit index signatures Aug 21, 2018
@brendankenny brendankenny changed the title core(tsc): remove more reliance on implicit index signatures core(tsc): remove more reliance on implicit index signature Aug 21, 2018
@brendankenny brendankenny changed the title core(tsc): remove more reliance on implicit index signature core(tsc): remove more reliance on implicit index signatures Aug 21, 2018
@brendankenny brendankenny merged commit 6a0d22e into master Aug 21, 2018
@brendankenny brendankenny deleted the i18ntsc branch August 21, 2018 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants