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

misc: remove all FR-COMPAT todos #13023

Merged
merged 2 commits into from
Sep 8, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions flow-report/src/summary/summary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ export const SummaryFlowStep: FunctionComponent<{
label: string,
hashIndex: number,
}> = ({lhr, label, hashIndex}) => {
// TODO(FR-COMPAT): Store report results globally.
const reportResult = useMemo(() => Util.prepareReportResult(lhr), [lhr]);

const screenshot = reportResult.gatherMode !== 'timespan' ? getScreenshot(reportResult) : null;
Expand Down Expand Up @@ -153,7 +152,6 @@ export const SummaryHeader: FunctionComponent = () => {
}
}

// TODO(FR-COMPAT): Pluralize UI strings.
const subtitleCounts = [];
if (numNavigation) subtitleCounts.push(`${numNavigation} navigation reports`);
if (numTimespan) subtitleCounts.push(`${numTimespan} timespan reports`);
Expand Down
1 change: 0 additions & 1 deletion flow-report/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ export function useDerivedStepNames() {
let numTimespan = 1;
let numSnapshot = 1;

// TODO(FR-COMPAT): Override with a provided step name.
return flowResult.lhrs.map((lhr, index) => {
const shortUrl = shortenUrl(lhr.finalUrl);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ class UnusedBytes extends Audit {
const [result, graph, simulator] = await Promise.all([
this.audit_(artifacts, networkRecords, context),
// Page dependency graph is only used in navigation mode.
// TODO(FR-COMPAT): Use dependency graph in timespan mode.
gatherContext.gatherMode === 'navigation' ?
PageDependencyGraph.request({trace, devtoolsLog}, context) :
null,
Expand Down Expand Up @@ -201,7 +200,6 @@ class UnusedBytes extends Audit {
}

/**
* TODO(FR-COMPAT): Rework opportunities to remove emphasis on `wastedMs`
* @param {number} wastedBytes
* @param {Simulator} simulator
*/
Expand Down
3 changes: 1 addition & 2 deletions lighthouse-core/fraggle-rock/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ function resolveArtifactsToDefns(artifacts, configDir) {
const coreGathererList = Runner.getGathererList();
const artifactDefns = artifacts.map(artifactJson => {
/** @type {LH.Config.GathererJson} */
// @ts-expect-error FR-COMPAT - eventually move the config-helpers to support new types
// @ts-expect-error - remove when legacy runner path is removed.
const gathererJson = artifactJson.gatherer;

const gatherer = resolveGathererToDefn(gathererJson, coreGathererList, configDir);
Expand Down Expand Up @@ -251,7 +251,6 @@ function initializeConfig(configJSON, context) {

configWorkingCopy = resolveExtensions(configWorkingCopy);

// TODO(FR-COMPAT): handle config plugins

const settings = resolveSettings(configWorkingCopy.settings || {}, context.settingsOverrides);
overrideSettingsForGatherMode(settings, context);
Expand Down
2 changes: 0 additions & 2 deletions lighthouse-core/fraggle-rock/gather/navigation-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,6 @@ async function _navigations({driver, config, requestedUrl, computedCache}) {
async function _cleanup({requestedUrl, driver, config}) {
const didResetStorage = !config.settings.disableStorageReset;
if (didResetStorage) await storage.clearDataForOrigin(driver.defaultSession, requestedUrl);

// TODO(FR-COMPAT): add driver.disconnect session tracking
}

/**
Expand Down
1 change: 0 additions & 1 deletion lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ class GatherRunner {
const session = driver.defaultSession;

// Assert no service workers are still installed, so we test that they would actually be installed for a new user.
// TODO(FR-COMPAT): re-evaluate the necessity of this check
await GatherRunner.assertNoSameOriginServiceWorkerClients(session, options.requestedUrl);

await prepare.prepareTargetForNavigationMode(driver, options.settings);
Expand Down
2 changes: 0 additions & 2 deletions lighthouse-core/gather/gatherers/css-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ class CSSUsage extends FRGatherer {
/** @param {LH.Crdp.CSS.StyleSheetRemovedEvent} sheet */
this._onStylesheetRemoved = sheet => {
// We can't fetch the content of removed stylesheets, so we ignore them completely.
// TODO(FR-COMPAT): Refactor use of CSSUsage artifact to not *always* rely on the content of stylesheets.
const styleSheetId = sheet.styleSheetId;
this._stylesheets = this._stylesheets.filter(s => s.header.styleSheetId !== styleSheetId);
};
Expand Down Expand Up @@ -84,7 +83,6 @@ class CSSUsage extends FRGatherer {
const session = context.driver.defaultSession;
const executionContext = context.driver.executionContext;

// TODO(FR-COMPAT): Do this only for snapshot once legacy runner is deprecated.
if (context.gatherMode !== 'timespan') {
await this.startCSSUsageTracking(context);

Expand Down
1 change: 0 additions & 1 deletion lighthouse-core/gather/gatherers/js-usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ class JsUsage extends FRGatherer {
usageByUrl[url] = scripts;
}

// TODO(FR-COMPAT): Enable this logic for legacy and navigation or make a separate artifact for url/scriptId mappings.
if (context.gatherMode !== 'navigation') {
this._addMissingScriptIds(usageByUrl);
}
Expand Down
2 changes: 0 additions & 2 deletions lighthouse-core/gather/gatherers/trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ class Trace extends FRGatherer {
* @param {LH.Gatherer.FRTransitionalContext} passContext
*/
async startSensitiveInstrumentation({driver, gatherMode}) {
// TODO(FR-COMPAT): read additional trace categories from overall settings?
// TODO(FR-COMPAT): check if CSS/DOM domains have been enabled in another session and warn?
await driver.defaultSession.sendCommand('Page.enable');
await driver.defaultSession.sendCommand('Tracing.start', {
categories: Trace.getDefaultTraceCategories().join(','),
Expand Down
1 change: 0 additions & 1 deletion lighthouse-core/lib/emulation.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ async function emulate(session, settings) {
* @return {Promise<void>}
*/
async function throttle(session, settings) {
// TODO(FR-COMPAT): reconsider if this should be resetting anything
if (settings.throttlingMethod !== 'devtools') return clearNetworkThrottling(session);

await Promise.all([
Expand Down
1 change: 0 additions & 1 deletion lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ class Runner {

/** @type {LH.RawIcu<LH.Result>} */
const i18nLhr = {
// TODO(FR-COMPAT): Make GatherContext a true base artifact.
// If GatherContext is not collected, then we are in a non-default pass of the legacy runner.
gatherMode: artifacts.GatherContext ? artifacts.GatherContext.gatherMode : 'navigation',
userAgent: artifacts.HostUserAgent,
Expand Down
3 changes: 0 additions & 3 deletions lighthouse-core/test/fraggle-rock/api-test-pptr.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ describe('Fraggle Rock API', () => {
expect(accessibility.score).toBeLessThan(1);

const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr);
// TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
expect(auditResults.length).toMatchInlineSnapshot(`80`);

expect(erroredAudits).toHaveLength(0);
Expand Down Expand Up @@ -129,7 +128,6 @@ describe('Fraggle Rock API', () => {
failedAudits,
notApplicableAudits,
} = getAuditsBreakdown(lhr);
// TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
expect(auditResults.length).toMatchInlineSnapshot(`48`);

expect(notApplicableAudits.length).toMatchInlineSnapshot(`6`);
Expand Down Expand Up @@ -194,7 +192,6 @@ describe('Fraggle Rock API', () => {

const {lhr} = result;
const {auditResults, failedAudits, erroredAudits} = getAuditsBreakdown(lhr);
// TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
expect(auditResults.length).toMatchInlineSnapshot(`154`);
expect(erroredAudits).toHaveLength(0);

Expand Down
1 change: 0 additions & 1 deletion types/lhr/flow.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import Result from './lhr';

/**
* The full output of a Lighthouse flow. Includes a series of Lighthouse runs.
* TODO(FR-COMPAT): Add flow specific metadata (e.g. interaction steps).
*/
export default interface FlowResult {
/** Ordered list of lighthouse results corresponding to a navigation, timespan, or snapshot. */
Expand Down