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

Lazy import and split SEO logger #416

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Lazy import and split SEO logger #416

merged 1 commit into from
Jan 31, 2023

Conversation

cartogram
Copy link
Contributor

This PR splits the SEO logging added in #400 to a lazily imported component. This allows for code-splitting and for it to be auto-disabled in production and auto-enabled in development (though this lives in user-land so it is easy to change this functionality if, for example, users wanted to enable via a query string).

For tophatting, you can run the demo-store via dev, you should see the logs. Then run the demo-store via preview and you should not see the logs. You can also filter the network requests to see the additional JS request for the logger code. See screenshot below:

Screenshot 2023-01-31 at 05 59 31

cc @gfscott

@cartogram cartogram requested a review from a team January 31, 2023 05:01
@cartogram
Copy link
Contributor Author

cartogram commented Jan 31, 2023

Build is currently failing, but should be fine once #415 is merged.

@github-actions
Copy link
Contributor

We detected some changes in packages/*/package.json or packages/*/src, and there are no updates in the .changeset.
If the changes are user-facing and should cause a version bump, run npm run changeset add to track your changes and include them in the next release CHANGELOG.
If you are making simple updates to examples or documentation, you do not need to add a changeset.

@cartogram
Copy link
Contributor Author

Additional context from chatting with @frandiox:

this only takes it out from the browser bundle, not for the worker bundle.

Ideally this PR from @frehner would allow different versions of hydrogen required to unlock the ability for us to ship separate versions of dev/prod. In the meantime this is better than nothing and a good reminder to revisit this when that time comes.

@cartogram cartogram merged commit be99be5 into v0.x-2022-10 Jan 31, 2023
@cartogram cartogram deleted the seo-env-split branch January 31, 2023 14:29
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.

2 participants