-
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): refactor to DevTools modules convention #16071
Conversation
import {runTraceEngine} from '../metrics/MetricTestUtils.js'; | ||
|
||
const {NetworkNode, CPUNode} = Lantern; | ||
const {Simulator, DNSCache} = Lantern.Simulation; |
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.
in some places I just do this so as not to fight line length limits
@@ -4,7 +4,7 @@ | |||
* SPDX-License-Identifier: Apache-2.0 | |||
*/ | |||
|
|||
import {BaseNode} from '../BaseNode.js'; | |||
import * as Lantern from '../lantern.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.
nit: the old way may still be valid since we're in the lantern/
directory already. Also would this cause any sort of circular dependency situation?
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.
ya know, the tests in CDT repo suggest relative imports like that are not preferred, but I do see non-test code doing that. so ok.
i'm not gonna keep spinning the wheels here so I'll keep it like this.
no circular issues here, everything executes fine. you have to be doing circular stuff in the root module scope for that.
This reverts commit a87e93a.
ref #15841
Makes
core/lib/lantern/lantern.js
the single entry point for all usages of Lantern in Lighthouse, tests, and types.