Base dispatching for events #34
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). nacelle-nextjs-example – ./examples/nextjs🔍 Inspect: https://vercel.com/nacelle/nacelle-nextjs-example/6R7MJHtAd8S3xudZuBnqwhLmrRDf nacelle-gatsby-example – ./examples/gatsby🔍 Inspect: https://vercel.com/nacelle/nacelle-gatsby-example/AbRuVZaSrCoN9FD7JYjx8mVNsrf4 [Deployment for 7a17789 failed] gamma-nova-jewelry – ./examples/nextjs🔍 Inspect: https://vercel.com/nacelle/gamma-nova-jewelry/yUWrdmviW6YV89evwV87PqXSKKAQ [Deployment for 7a17789 failed] |
There was a problem hiding this comment.
Nice use of a reducer!
There are a few changes that will need to be made to conform with the Rules of Hooks, and some miscellaneous things that I've noted in the comments. Here is a diff of the changes required for examples/nextjs/pages/products/[handle].js:
Do you have the ESLint VSCode extension? If so, it should be pointing out some of the ESLint errors, like this:
It would be great if you could also set up more events (in addition to PRODUCT_VIEW) so that each event shown in the reducer gets triggered.
Lastly, if you could please add some screenshots demonstrating the functionality, or better yet, steps to follow to test the functionality, that would be very helpful.
| const [eventLog, dispatchEvent] = useReducer(eventReducer, []); | ||
|
|
||
| useEffect(() => { | ||
| if (eventLog.length > 0 && process.browser) { |
There was a problem hiding this comment.
process.browser is deprecated in Next.js - instead, use typeof window !== 'undefined'
| @@ -15,6 +15,11 @@ const ProductDetail = ({ product }) => { | |||
| return <div>Loading...</div>; | |||
| } | |||
|
|
|||
| const { dispatchEvent } = useContext(EventLogContext); | |||
There was a problem hiding this comment.
EventLogContextisundefinedand needs to be importeduseContextmust go above theifblock, since hooks can't be called conditionally.
| @@ -15,6 +15,11 @@ const ProductDetail = ({ product }) => { | |||
| return <div>Loading...</div>; | |||
| } | |||
|
|
|||
| const { dispatchEvent } = useContext(EventLogContext); | |||
| useEffect(() => { | |||
There was a problem hiding this comment.
useEffect must go above the if block, since hooks can't be called conditionally.
| const eventReducer = (state, action) => { | ||
| if ( | ||
| action.type === 'PAGE_VIEW' || | ||
| action.type === 'PRODUCT_VIEW' || | ||
| action.type === 'ADD_TO_CART' || | ||
| action.type === 'REMOVE_FROM_CART' || | ||
| action.type === 'CHECKOUT_INIT' | ||
| ) { | ||
| return [...state, action]; | ||
| } | ||
| return state; | ||
| }; |
| console.log('PAGE VIEW EVENT FIRED'); | ||
| break; | ||
| case 'PRODUCT_VIEW': | ||
| console.log('PRODUCT_VIEW EVENT FIRED', event.payload); |
There was a problem hiding this comment.
it's probably good to JSON.stringify(event.payload, null, 2) so that it doesn't show [object Object]
Thanks for taking a look Nick I fixed all this! I'm happy to know I wasn't way off with this way of doing things |
| @@ -1,11 +1,17 @@ | |||
| import React from 'react'; | |||
| import React, { useContext } from 'react'; | |||
There was a problem hiding this comment.
useEffect also needs to be imported:
There was a problem hiding this comment.
Good catch! Strange it was working when I tested it with logs...
Should be good now thank you!
Story: DRAMS-299
Type of Change
What is being changed and why?
How to Test