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

Remix v2 #1289

Merged
merged 49 commits into from
Oct 26, 2023
Merged

Remix v2 #1289

merged 49 commits into from
Oct 26, 2023

Conversation

frandiox
Copy link
Contributor

@frandiox frandiox commented Aug 31, 2023

FYI We'll release v2 support in our next major bump to avoid breaking changes for users in minor/patch releases.


This is just to track the required changes for supporting Remix v2.

I had to add a few @ts-ignore due to a bug in the types: remix-run/remix#7246 Fixed!
Should be fixed for v2 stable I think.

Todos:

Note: the preview environment in this PR shows the demo-store and it has unrelated errors due to the new CSP

@github-actions

This comment has been minimized.

@@ -59,7 +59,7 @@ export async function loader({params, request, context}: LoaderArgs) {
}

if (!product.selectedVariant) {
return redirectToFirstVariant({product, request});
throw redirectToFirstVariant({product, request});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes returned types work better, and I think it doesn't change the behavior, right?

Comment on lines +63 to +66
export const useRootLoaderData = () => {
const [root] = useMatches();
return root?.data as SerializeFrom<typeof loader>;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm adding this small abstraction to enhance types. Thoughts?

}
```

This way, you can import `useRootLoaderData()` anywhere in your app and get the correct type for the data returned by the root loader.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is cool, I didn't know about it.

Co-authored-by: Bret Little <bret.little@shopify.com>
@wizardlyhel wizardlyhel mentioned this pull request Oct 20, 2023
12 tasks
@frandiox frandiox requested a review from a team October 22, 2023 23:00
Copy link
Contributor

@wizardlyhel wizardlyhel left a comment

Choose a reason for hiding this comment

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

I'm good with useRootLoaderData

I think we should note breaking change in changeset?

  • V2_MetaFunction rename to MetaFunction
  • LoaderArgs rename to LoaderFunctionArgs

@frandiox
Copy link
Contributor Author

I think we should note breaking change in changeset?

I had links to the Remix release notes in the demo-store changeset but I've now added the same links to the other packages. Also added a couple of common examples 👍

@frandiox frandiox merged commit 66a4857 into main Oct 26, 2023
9 checks passed
@frandiox frandiox deleted the fd-remix-2 branch October 26, 2023 05:27
@frandiox frandiox restored the fd-remix-2 branch October 26, 2023 07:20
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.

3 participants