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: remove config navigations #15397

Merged
merged 13 commits into from
Oct 18, 2023
Merged

core: remove config navigations #15397

merged 13 commits into from
Oct 18, 2023

Conversation

adamraine
Copy link
Member

@adamraine adamraine commented Aug 22, 2023

Config navigations were moved to internal stuff in 10.0: #13881. It was our plan to remove them completely once the legacy path had been removed completely. This PR fulfills the end goal of that plan.

From a user perspective, the only difference should be that we perform 1 about:blank jump at the start instead of 2. The rest of this is just removing technical debt.

Having just one about:blank jump makes it easy to fix #15487 as well.

"pauseAfterFcpMs": 1000,
"pauseAfterLoadMs": 1000,
"networkQuietThresholdMs": 1000,
"cpuQuietThresholdMs": 1000,
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are these changing?

Copy link
Member Author

Choose a reason for hiding this comment

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

We apply overrides to these default values when using DT throttling. Previously, this was applied to the values on the fake navigation object constructed during initializeConfig.

Now that the fake navigation object is removed, we have to apply the overrides to the settings object directly.

/**
* @param {LH.Gatherer.GatherMode} gatherMode
* @param {LH.Config=} config
* @param {LH.Flags=} flags
* @return {Promise<{resolvedConfig: LH.Config.ResolvedConfig, warnings: string[]}>}
Copy link
Member Author

Choose a reason for hiding this comment

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

There are no config warnings anymore but there could be in the future I guess. Shouldn't be too hard to add this if we ever want them again.

@adamraine adamraine merged commit d29a447 into main Oct 18, 2023
26 of 27 checks passed
@adamraine adamraine deleted the rm-config-navigations branch October 18, 2023 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Provided URL did not match initial navigation URL"
3 participants