Replies: 2 comments 3 replies
-
I may add that
is inaccurate as it should be
or perhaps the dynamicLink function should never be called in the first place if the loader throws? |
Beta Was this translation helpful? Give feedback.
3 replies
-
DynamicLinks are being removed in favor of using the v2 of MetaFunction. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
DynamicLinks
is usually placed within theroot
HTML template and there's no fail-safe in case the user input function throws for whatever reason. This might not matter to most people since theErrorBoundary
handles that failure, but, in our case, ourroot
cannot fail because we are cheating remix and relying on the root loader data inroot
within the root ErrorBoundary. The root loader data in our case is just thegetLoadContextData
which is always available.This is what it kinda looks like:
With the following code in mind you can see that if
DynamicLinks
throws, the error boundary itself won't have what it needs which would cause it to fail as well. Wrapping that in the old school react ErrorBoundary class component was supposed to be the work-around to avoid patching and/or opening up a discussion withremix-utils
but it doesn't work as intended for whatever reason.So.. the question is, what your opinion @sergiodxa on adding a
try catch
around the userfn
call in https://github.com/sergiodxa/remix-utils/blob/main/src/react/dynamic-links.tsx to ensure that it doesn't throw. I'm going to assume that this is intentional because theErrorBoundary
is there for this exact reason.Beta Was this translation helpful? Give feedback.
All reactions