Skip to content

✨Add core option to skip asset discovery#1038

Merged
wwilsman merged 2 commits intomasterfrom
ww/core-skip-discovery-option
Aug 17, 2022
Merged

✨Add core option to skip asset discovery#1038
wwilsman merged 2 commits intomasterfrom
ww/core-skip-discovery-option

Conversation

@wwilsman
Copy link
Copy Markdown
Contributor

What is this?

While asset discovery is usually necessary for taking DOM snapshots, sometimes we don't actually need to take DOM snapshots at all, such as with the upload command. For these circumstances, downloading a browser for asset discovery that won't ever be used is unnecessary.

For the upload command, we currently have a 2-part approach to skipping asset discovery. First, we utilize a @percy/cli-command specific option, discoveryFlags, to suppress asset discovery flags from being accepted. Second, we provide the percy.start() method with an option to skip launching a browser, which in-turn prevents the browser download completely.

For future projects, this 2-part solution isn't possible, and even now seems like a workaround for a missing feature. This PR adds that missing feature: an option to skip asset discovery altogether with a new skipDiscovery core option.

The new skipDiscovery option actually works nicely with the existing dryRun option. The dryRun option already skips asset discovery along with disabling uploads. With the new skipDiscovery option, some logic branches were changed from checking for dryRun. Now, besides a few specific places for logging, dryRun is equivalent to skipDiscovery && skipUploads.

Extra logic was added to prevent snapshot captures when asset discovery is disabled since capturing the DOM requires a browser which skipDiscovery specifically prevents from being launched. If any normal DOM snapshots are received, they simply bypass asset discovery completely.

This option is not currently exposed publicly in any way. Only the upload command and specific future SDKs will utilize this feature internally.

This also removes the need to prevent a browser from launching when starting percy
@wwilsman wwilsman added the ✨ enhancement New feature or request label Aug 17, 2022
@wwilsman wwilsman requested a review from Robdel12 August 17, 2022 22:10
Copy link
Copy Markdown
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

🏁

@wwilsman wwilsman merged commit ba2d3e8 into master Aug 17, 2022
@wwilsman wwilsman deleted the ww/core-skip-discovery-option branch August 17, 2022 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants