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.

Sign up for GitHub

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

Jump to bottom

fix(gatsby): Use path prefix when loading slice data #37202

Merged
merged 2 commits into from Dec 7, 2022
Merged

fix(gatsby): Use path prefix when loading slice data #37202

merged 2 commits into from Dec 7, 2022

Conversation

mudge
Copy link
Contributor

@mudge mudge commented Dec 7, 2022

Description

If an asset or path prefix has been set and prefix paths is enabled, slice data should loaded using that prefix.

Documentation

As slice data are non-HTML resources, they should follow the existing documentation for asset prefixes in https://www.gatsbyjs.com/docs/how-to/previews-deploys-hosting/asset-prefix/

Related Issues

Addresses #37143

@mudge
fix(gatsby): Use path prefix when loading slice data
ba922ae 
As per the discussion in
#37143, if an asset or
path prefix has been set and prefix paths is enabled, slice data should
loaded using that prefix.
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 7, 2022
@pieh pieh self-assigned this Dec 7, 2022
@pieh pieh added topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 7, 2022
@mudge
Copy link
Contributor Author

mudge commented Dec 7, 2022

There don’t seem to be any tests around the loadSliceDataJson function (or for page-data with a prefix) so I tested this by modifying it for a site and running a build both with and without a prefix enabled.

@pieh
Copy link
Contributor

pieh commented Dec 7, 2022

There don’t seem to be any tests around the loadSliceDataJson function (or for page-data with a prefix) so I tested this by modifying it for a site and running a build both with and without a prefix enabled.

I added some e2e/integration style tests to test that in the end both page-data and slice-data are using asset prefixes in 515b7d1

Let's wait for CI to finish and if all is still green will go ahead and merge

pieh
pieh approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

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

Thank you!

@pieh pieh merged commit b346071 into gatsbyjs:master Dec 7, 2022
@mudge mudge deleted the topics/slice-data-with-path-prefix branch December 7, 2022 15:34
@pieh
Copy link
Contributor

pieh commented Dec 7, 2022 •
edited

Published gatsby@next with this change: gatsby@5.3.0-next.4 - this will be published in @latest in few days, but can be used already if needed with @next until then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pieh pieh pieh approved these changes

Assignees

@pieh pieh

Labels
topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mudge @pieh