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

Remove support for custom responses #1340

Merged
merged 12 commits into from
May 25, 2022
Merged

Conversation

jplhomer
Copy link
Contributor

Description

Fixes #1267.

The concept of "custom responses" was introduced early in Hydrogen's development, prior to API routes. However, we haven't seen this functionality used beyond simple sitemap.xml or robots.txt purposes — both of which can be handled by API routes.

This PR removes response.send() as well as the custom body, which only existed to support custom responses.


Before submitting the PR, please make sure you do the following:

  • Read the Contributing Guidelines
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123)
  • Update docs in this repository according to your change
  • Run yarn changeset add if this PR cause a version bump based on Keep a Changelog and adheres to Semantic Versioning

@jplhomer jplhomer requested a review from a team May 24, 2022 18:56
query: QUERY,
variables: {
language: languageCode,
language: 'EN',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that, unless the developer provides a defaultLanguageCode in their hydrogen.config.js, we default to 'EN' anyway. If the developer wishes to customize language code based on e.g. domain, they can add functionality here to do so, since it's part of the demo store template code.

Copy link
Contributor

@mcvinci mcvinci left a comment

Choose a reason for hiding this comment

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

Thanks for removing the docs around this as well, @jplhomer!

@mcvinci
Copy link
Contributor

mcvinci commented May 24, 2022

Could we also update framework/seo.md which has references to sitemap.xml.server and robots.txt.server being a JSX files (now JS files)?

Copy link
Contributor

@frandiox frandiox left a comment

Choose a reason for hiding this comment

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

LGTM! We can also make some existing functions like onAllReady or prepareForStreaming synchronous now that this code is removed.

@jplhomer jplhomer merged commit 631832e into v1.x-2022-07 May 25, 2022
@jplhomer jplhomer deleted the jl-remove-custom-responses branch May 25, 2022 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove custom response support from Hydrogen
3 participants