Possible improvement to core#capture

[nathanshelly]

Hi again!

I'm trying to use the Percy CLI to test snapshots with two extra requirements that the current capture API doesn't seem to account for. One of those cases is pages behind logins and the other is an in-house implementation of frontend deploy previews for PRs.

The login case requires programmatically navigating to our company's login page and entering credentials before being redirected back to the page on which I actually want to take the screenshot. The deploy preview case requires navigating to a programmatically generated URL that sets a cookie on the client which then enables viewing the specific frontend assets associated with the corresponding PR.

At one point I was hoping to use the execute argument but if that execute function navigates to a separate page I get the following error:

[percy] Encountered an error for page: ...
[percy] Error: Evaluation failed: ReferenceError: PercyDOM is not defined
    at __puppeteer_evaluation_script__:5:20

Looking at the code this makes sense since the SDK injects @percy/dom before the execute function runs so navigating away from the page loses that injection.

Do you have a suggestion for handling this use case? If there is no straightforward answer would you all be open to a PR introducing a new argument something like executeBeforePercySetup to run any login/other setup logic before that injection?

[wwilsman]

Hello again @nathanshelly!

I've moved this issue to the new repo since it relates directly to it.

If you can provide authorization headers via the headers option, that may also work to bypass the login screen entirely.

percy.capture({
  name: 'Behind Login',
  url: 'http://localhost:3000/protected',
  headers: {
    Authorization: 'XXXXXX'
  }
})

However, this issue did bring to my attention that if any page navigation occurs inside of the execute function, then it will also result in the error you encountered. Rather than introduce another execute option, I've opted to move the injection step to after execute is called. I also noticed there was another nuance where if the page was redirected, the incorrect URL was being provided to the snapshot method.

I've addressed both of those issues in PR #99. If you can confirm that this would solve your issue, I'll go ahead and merge it.

[nathanshelly]

Whoops sorry just blindly created it in the same repo as the previous issue, thanks for moving!

That resolution sounds perfect. Thank you so much for the continued incredible support!

[nathanshelly]

Following up here I was hopeful I would be able to install the latest, unreleased changes including this fix via something like this in my package.json:

"@percy/cli": "git+https://github.com/percy/cli.git#b6bceeeff58f35f379dcd2141db55259344bb3d1",

Unfortunately this fails with an error message about a missing version field. This seems to be due to a limitation of yarn with monorepo structures like this repo where it's attempting to install the top-level package.json which has no version field instead of the nested packages/cli folder that I'd need. Any chance you'd be willing to cut a new beta release so I could use the fix?

[wwilsman]

Sorry about the delay in releasing! I meant to do that asap, but wasn't near a computer when I merged the PR from mobile. Then my brain turned off work mode all weekend and I forgot.

I'll have a release cut shortly! 👍

[wwilsman]

Released 1.0.0-beta.25 🎉

[nathanshelly]

No worries whatsoever, thanks!