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

Move Demo Store to templates folder #1132

Merged
merged 3 commits into from
May 3, 2022
Merged

Move Demo Store to templates folder #1132

merged 3 commits into from
May 3, 2022

Conversation

cartogram
Copy link
Contributor

Fixes Shopify/hydrogen#817

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

@cartogram cartogram requested a review from a team April 26, 2022 06:49
@caution-tape-bot
Copy link

We noticed that this PR either modifies or introduces usage of the dangerouslySetInnerHTML attribute, which can cause cross-site scripting (XSS) vulnerabilities when user controlled values are passed in.
We recommend reviewing your code to ensure that this is what you intended to use and that there is not a safe alternative available.

Docs are available here.

If unavoidable, we reccomend using an HTML sanitizer like DOMPurify to sanitize content before
rendering it as HTML.

If you have any questions or are unsure about how to move forward with this, ping #help-appsec and we would
be happy to help you out! cc: @Shopify/xss-extermination-squad

jplhomer
jplhomer approved these changes Apr 26, 2022
View reviewed changes
Copy link
Contributor

@jplhomer jplhomer 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!!!

frandiox
frandiox approved these changes Apr 26, 2022
View reviewed changes
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.

Thanks!!

With this change, should we also move packages/playground/** to examples? Or even to a different root folder? It doesn't feel like a package 🤔

@cartogram cartogram force-pushed the @cartogram/move-templates branch from f058c19 to d2a6e69 Compare April 27, 2022 06:00
@cartogram
Copy link
Contributor Author

@frandiox yeah I agree, playground doesn't feel like it is in the right place.

My thinking here is that we remove that folder entirely and more towards declarative sandboxed e2e apps that are generated before each test runs and destroyed after. The fixture approach won't scale as well and we commonly re-use fixtures across different tests which is not ideal. What do you think? I have a few things started around scaffolding sandbox apps so if you agree with that approach I can put together a PR.

@cartogram cartogram force-pushed the @cartogram/move-templates branch from d2a6e69 to 9b79c09 Compare April 28, 2022 14:00
@cartogram cartogram force-pushed the @cartogram/move-templates branch from 9b79c09 to 10b3545 Compare May 3, 2022 21:08
@cartogram cartogram merged commit 5f39c61 into v1.x-2022-07 May 3, 2022
@cartogram cartogram deleted the @cartogram/move-templates branch May 3, 2022 21:26
@cartogram cartogram mentioned this pull request May 3, 2022
Closed
blittle added a commit that referenced this pull request May 4, 2022
@blittle
Merge branch 'v1.x-2022-07' into bl-countries-session
afc036a 
* v1.x-2022-07: (95 commits)
  [ci] release v1.x-2022-07 (#1170)
  Try ignoring hello-world to see if it will get bumped
  Don't consider examples part of the workspace (#1202)
  Fix headers on oxygen (#1201)
  Add bot user agents for Seoradar and Adresults, resolves #1199 (#1200)
  Fix changeset
  updates to docker deploy documentation to resolve run issues (#1196)
  Upgrade body-parser (#1162)
  Fix path for deployments
  Adds ability to add more than one cookie per response (#1161)
  Move Demo Store to templates folder (#1132)
  Avoid additional div element (#1191)
  Whoops this should only be patch
  Adds preconnect <link> for CDN (#1160)
  Bump ejs from 3.1.6 to 3.1.7 (#1147)
  Fix scroll restoration when server props are changed (#1152)
  Typo
  Fixes #1165 by making a missing alt tag a console warning (#1167)
  Remove concurrency directive for Oxygen deployments
  Fix hydrogen-ui dev and build issues (#1169)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frandiox frandiox frandiox approved these changes

@jplhomer jplhomer jplhomer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move hydrogen-template-default to templates folder instead of examples
3 participants
@cartogram @jplhomer @frandiox