-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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(lantern): move NetworkRequest to lib/dependency-graph #15842
Conversation
5f77645
to
77b6952
Compare
77b6952
to
00de512
Compare
00de512
to
d82a880
Compare
@@ -4,13 +4,12 @@ | |||
* SPDX-License-Identifier: Apache-2.0 | |||
*/ | |||
|
|||
import * as LH from '../../../types/lh.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
@@ -5,6 +5,7 @@ | |||
*/ | |||
|
|||
import * as LH from '../../../../types/lh.js'; | |||
import {NetworkRequest} from '../network-request.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would /** @typedef {import('../network-request.js').NetworkRequest} NetworkRequest */
also work? I would prefer that to getting all these "unused" imports and then ignoring the eslint rule.
LH
only needed a physical import because LH
is a module and jsdoc doesn't support that (yet). NetworkRequest
is an actual type that we can assign using jsdoc.
ref #15841
NetworkRequest is used to model the network inside the Lantern graph and simulator. Instead of attempting to recreate a very similar interface that continues to meet the needs of all our use cases in Lighthouse, let's just move the entire NetworkRequest to lantern. Consumers of the library will need to create these on their own to make a graph, and how they do that (CDT logs or trace) is irrelevant.
We may possibly revisit the network interface in Lantern down the line.