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

feat(reference-stores): nuxt #70

Merged
merged 18 commits into from
Feb 7, 2022
Merged

Conversation

alexcasche
Copy link
Contributor

@alexcasche alexcasche commented Feb 1, 2022

WHY are these changes introduced?

Story [LS-1451] - Create a Nuxt.js Reference Store for Nacelle v2.

WHAT is this pull request doing?

  • Adds a new directory called reference-stores
  • Adds a Nuxt.js Reference store to directory
  • Uses @nacelle/storefront-sdk

How to Test

  1. Navigate to Nuxt.js Reference Store cd reference-stores/nuxt
  2. Create .env and add the following
# Nacelle
NACELLE_STOREFRONT_ENDPOINT="https://storefront.api.development.nacelle.com/graphql/v1/spaces/ckya9k7cw1587373nx3m6s3lbn6"
NACELLE_STOREFRONT_TOKEN="45da574e-798c-4582-8405-4ae85c11d621"
NACELLE_STOREFRONT_LOCALE="en-US"

# Shopify Checkout (for details, see: https://www.npmjs.com/package/@nacelle/shopify-checkout)
MYSHOPIFY_DOMAIN="pepper-wood-apparel.myshopify.com"
SHOPIFY_STOREFRONT_CHECKOUT_TOKEN="477e3f771eb126e58db487428cd34948"
STOREFRONT_API_VERSION="2022-01"
  1. Run npm run dev or npm run build and npm run start.
  2. Click around the site to ensure all routes work as expected
  3. Add a product to cart, and proceed to checkout. ** Some products were indexed without source Entry Ids. This is causing checkout to break, but it is not a bug.

Important Notes

  1. @nuxt/image@0.5.0 is installed. The newer releases have a bug that causes dev server to crash when saving nuxt.config.js file. Here is the issue:
    Receiving Cannot read properties of undefined (reading 'get') on version 0.6.1 nuxt/image#481

  2. We will need a Vercel deployment for the nuxt reference store repo. Rather than create a new one, I think we should convert the old one. ** I do not have permission to update the git settings in Vercel.

@vercel
Copy link

vercel bot commented Feb 1, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

nacelle-js-nuxt-starter – ./starters/nuxt

🔍 Inspect: https://vercel.com/nacelle/nacelle-js-nuxt-starter/8uyLdnjLr97j5QDyynKfKhQhorkX
✅ Preview: https://nacelle-js-nuxt-starter-git-ls-1451-nuxt-referen-9bb75c-nacelle.vercel.app

nacelle-js-gatsby-starter – ./starters/gatsby

🔍 Inspect: https://vercel.com/nacelle/nacelle-js-gatsby-starter/Gkfzy1ppydomiWY2Sq4gs7rZaAFE
✅ Preview: https://nacelle-js-gatsby-starter-git-ls-1451-nuxt-refer-b1b120-nacelle.vercel.app

[Deployment for 93302fb failed]

@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2022

Coverage report for packages/shopify-checkout

Total coverage

Status Category Percentage Covered / Total
🟢 Statements 99.52% 206/207
🟢 Branches 90.53% 86/95
🟢 Functions 100% 52/52
🟢 Lines 99.6% 496/498

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Report generated by 🧪jest coverage report action from 3ad13ae

'@nuxt/image',
'@nuxtjs/svg',
'@nuxtjs/pwa',
'@nuxt/postcss8'
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there a specific reason that we dropped @nuxt/tailwindcss for a manual setup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdarrik yea it seems like it has been abandoned. Nick was also seeing some error testing. The manual setup is what tailwind recommends for Nuxt

Copy link
Contributor

@NWRichmond NWRichmond Feb 2, 2022

Choose a reason for hiding this comment

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

Yeah. The version of tailwind that @nuxtjs/tailwindcss uses was conflicting with some of the tailwind plugins we're using.

For more context on the Nuxt module's state of neglect: nuxt-modules/tailwindcss#406.

Copy link
Contributor

@jasonredick jasonredick left a comment

Choose a reason for hiding this comment

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

Looks great! I assume the line items stacking into one when adding to the cart is because of the empty variant id issue. I did have a small change on the cross sell items. As well as some potential updates on some search files, depending on what the goal of search is in our reference stores.

reference-stores/nuxt/components/header/HeaderSearch.vue Outdated Show resolved Hide resolved
reference-stores/nuxt/components/search/SearchFilters.vue Outdated Show resolved Hide resolved
reference-stores/nuxt/utils/filterCatalog.js Outdated Show resolved Hide resolved
@alexcasche alexcasche marked this pull request as draft February 3, 2022 14:51
Copy link
Contributor

@NWRichmond NWRichmond left a comment

Choose a reason for hiding this comment

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

Everything seems to be working as expected:

  • Landing page
  • Collection PLP
  • PDP
  • Cart
  • Cart cross-sells
  • General pages
  • Search
  • Filtering
  • Checkout
v2.Nuxt.Reference.Store.demo.LS-1451--nuxt-reference-store.mp4

Don't sweat the starters/gatsby Vercel deploy preview failure - while the source of that issue is unclear, it appears to be an issue on main that only presented today, for reasons unknown.

Thanks so much for the commitment and interaction on this PR!

@alexcasche alexcasche merged commit 1d18882 into main Feb 7, 2022
@alexcasche alexcasche deleted the LS-1451--nuxt-reference-store branch February 7, 2022 16:26
@KrisQ KrisQ added the enhancement New feature or request label Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants