Problem/Motivation

Gatsby Cloud (and potentially other preview providers) don't always need the /__refresh part of the URL. This module automatically assumes this is needed for preview URLs. You can see this here:

https://git.drupalcode.org/project/gatsby/-/blob/8.x-1.x/src/GatsbyPrevi... and https://git.drupalcode.org/project/gatsby/-/blob/8.x-1.x/modules/gatsby_...

This used to work in Gatsby Cloud but it was an undocumented approach that now appears to be unsupported.

Steps to reproduce

Try to get preview working in Gatsby Cloud.

Proposed resolution

- Remove the /__refresh part of the URL for preview builds (in the areas of code linked above)
- Update the documentation on the preview webhook field to be clear that the user may now need to add this
- This could technically be a breaking change, so it should probably automatically add /__refresh in an update hook so most sites will continue to work after the update

Issue fork gatsby-3204395

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

codekarate created an issue. See original summary.

AJV009’s picture

AJV009
Primary language English
Location Maharashtra
CreditAttribution: AJV009 as a volunteer and at QED42 for Drupal India Association commented

Cool, even I noticed this! My site did break once it tried to call '__refresh' I thought it was because of my incomplete gatsby setup[ and maybe it already supported this '__redi....', so its a bug? would love to get it fixed asap!

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Issue tags: +Needs tests

Because the code uses a public static (for now) this should be easy to test.

codekarate’s picture

codekarate CreditAttribution: codekarate commented
Assigned: Unassigned » codekarate

kyle_mathews made their first commit to this issue’s fork.

kyle_mathews opened merge request !17

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Active » Needs review

Pushed test and coding standards fixes, added the missing schema and default value.

Made the support for multiple values with CSV explicit.

Added an update path.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Needs review » Fixed

  • larowlan committed f27acb3 on 8.x-1.x 
    Issue #3204395 by kyle_mathews, larowlan, codekarate: Don't...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Issue tags: -Needs tests

Got the Drupal CI notification that this went in, but there's a PHPCS issue.

Fixed it and pushed that commit along with a config file.

For running locally, you can use /path/to/drupalroot/vendor/bin/phpcs . from inside the gatsby module folder.

/path/to/drupalroot/vendor/bin/phpcbf . will fix things that can be auto-fixed.

codekarate’s picture

codekarate CreditAttribution: codekarate at Gatsby commented
Status: Fixed » Closed (fixed)

Thanks for the quick fix @larowlan!

This is working for me so I'm marking it completed.