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

Vendure - storefront Remix starter - code review feedback #71

Open
8 tasks
hans-rollingridges-dev opened this issue Feb 3, 2024 · 2 comments
Open
8 tasks

Comments

@hans-rollingridges-dev
Copy link

hans-rollingridges-dev commented Feb 3, 2024

This issue concerns code review feedback for the Vendure storefront Remix starter. The feedback is intended to improve the Remix starter by providing a to-do list.

If you would like to be assigned to one of these tasks, please leave a comment.

When assigned, each task is further refined and broken down into subtasks related to a specific small unit of work.

Depending on the interest in taking on tasks, the list can be expanded with more improvements!

  • Improve - global app data - provisioning and type safety
    Description

    Global app data provision via outlet and component properties has become less efficient. In this case, applying a root context provider is a good solution. Because the root context can be provided to any child component via a hook. As a bonus, a custom useRootContext hook ensures consistent global app data type safety across every component.

    • Implement root context provider

      Code example /app/contexts/root-context.ts
      import { type Dispatch, type SetStateAction, createContext, useContext } from "react";
      
      type RootContextType = {
          cartOpenState: [boolean, Dispatch<SetStateAction<boolean>>];
          orderService: OrderServiceType;
      }
      
      export const RootContext = createContext<RootContextType | null>(null);
      
      export const useRootContext = () => {
          const rootContext = useContext(RootContext);
      
          if (!rootContext) {
              throw new Error(
                  "useRootContext has to be used within <RootContext.Provider>"
              );
          }
      
          return rootContext;
      };
      Code example /app/root.tsx
      import { RootContext } from "~/contexts/RootContext";
      
      export default function App() {
          const cartOpenState = useState(false);
          const orderService = useOrderService();
      
          return (
              <RootContext.Provider value={{
                  cartOpenState,
                  orderService,
              }} >
                  <html>
      
                  </html>
              </RootContext.Provider>
          );
      }
    • Refactor outlets and components to apply the useRootContext hook.

      Code example /app/components/Header.tsx
      import { useRootContext } from '~/contexts/RootContext'
      
      export default function Header() {
          const {
              cartOpenState: [cartOpen, setCartOpen],
              orderService,
          } = useRootContext();
      
          return (
              <>
                  <Cart open={cartOpen} setOpen={setCartOpen} />
      
                  <header>
      
                  </header>
              </>
          )
      }
  • Add checkout end-to-end tests
    Description

    Before refactoring the checkout form, it is good to first add an end-to-end test. After looking at the code and some manual testing, you must be able to add failing test cases.

    • Test form submission as a logged in user, next as a guest with the same email address.
    • Test changing customer form fields without leaving input focus and clicking immediately the submit button.

    Guide

    The official Remix templates are a great start as they include testing and Github workflows. Create in a temporary folder a new Remix app with the blues-stack template.

    npx create-remix@latest --template remix-run/blues-stack

    Follow the README.md instructions and run the end-to-end tests. Next, copy the relevant code in your the Vendure Remix starter project and add the failing test cases.

  • Improve checkout form - on blur - submission

  • Improve checkout form client/server side validation

    • Implement zod client/server side form validation
  • Improve checkout form client/server side error handling.

    • Show a clear client side error message
      • on client/server form validation failure
      • on server error, e.g. Vendure - GraphQL - shop API failure
  • Support default configurable guest checkout strategies

    • Show a login button when guest email is not available
  • Improve Remix route revalidation

  • Improve multilingual support

@kyunal
Copy link
Collaborator

kyunal commented Feb 4, 2024

First of all, thanks for this comprehensive list including code examples and references - very much appreciated! A lot of these stem from just building on what's already there instead of refactoring where appropriate to be more in-line with common practices, patterns or simply what's best.

I won't really be able to address any of these soon unfortunately. That said, I believe it would make sense to convert these into single issues and label them accordingly, so others could chime in and provide PRs for these matters. If you want you can raise these issues, else I could do it too.

@hans-rollingridges-dev
Copy link
Author

hans-rollingridges-dev commented Feb 4, 2024

It's actually easy to turn these tasks into separate issues! After refining the tasks, I will hover in the top right corner and click "convert to issue". Also, if there is enough interest, I would schedule these improvements in a 'Road Map' Github project.

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

No branches or pull requests

2 participants