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

Support Rails 7 #1354

Merged
merged 10 commits into from
Jan 28, 2022
Merged

Support Rails 7 #1354

merged 10 commits into from
Jan 28, 2022

Conversation

gonzaloriestra
Copy link
Contributor

@gonzaloriestra gonzaloriestra commented Jan 26, 2022

Closes #1339

What this PR does

  • Adds support to work with Rails 7 projects
  • Updates the install generator to work with Importmap instead of Webpack
  • Fix the regex to properly whitelist Ngrok tunnel hostnames

Reviewer's guide to testing

  1. Ensure you have Rails 7: rails -v
  2. rails new my_app
  3. bundle add shopify_app --git 'git://github.com/Shopify/shopify_app.git' --branch 'rails-7'
  4. rails generate shopify_app
  5. rails db:migrate
  6. ngrok http 3000 and copy the HTTPS URL
  7. Create a new app in Partners, adding the Ngrok URL followed by /auth/shopify/callback in the Allowed redirection URLs field
  8. Copy the generated api key and secret
  9. SHOPIFY_API_KEY=x SHOPIFY_API_SECRET=y rails server
  10. Go to the Ngrok URL, install the app on a store and check that it works

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • For security fixes, the Disclosure Policy must be followed.

@@ -9,6 +9,10 @@
/>
<% unless with_cookie_authentication? %> <script>
document.addEventListener("DOMContentLoaded", async function() {
<% if ShopifyApp.use_importmap? %>
await import("lib/shopify_app")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this the best way to load shopify_app.js? It requires the DOM content to be loaded, but we need to run it before this point.

Choose a reason for hiding this comment

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

Both, await and import are supported by most of the browsers so I'd say it's fine.

@gonzaloriestra gonzaloriestra marked this pull request as ready for review January 26, 2022 11:48
.nvmrc Show resolved Hide resolved
@@ -9,6 +9,10 @@
/>
<% unless with_cookie_authentication? %> <script>
document.addEventListener("DOMContentLoaded", async function() {
<% if ShopifyApp.use_importmap? %>
await import("lib/shopify_app")

Choose a reason for hiding this comment

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

Both, await and import are supported by most of the browsers so I'd say it's fine.

@attack attack self-requested a review January 27, 2022 18:29
Copy link

@attack attack left a comment

Choose a reason for hiding this comment

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

No issues discovered via tophat.

@gonzaloriestra gonzaloriestra merged commit a7a3bfe into master Jan 28, 2022
@gonzaloriestra gonzaloriestra deleted the rails-7 branch January 28, 2022 10:40
@kirillplatonov
Copy link
Contributor

Amazing update! 👍🔥

@shopify-shipit shopify-shipit bot temporarily deployed to rubygems February 1, 2022 16:56 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems April 6, 2022 18:22 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems-v18 June 3, 2022 14:59 Inactive
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.

Gem to support rails 7
4 participants