Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(v2_meta): hint data as being potentially undefined #6231

Merged
merged 3 commits into from
May 5, 2023

Conversation

machour
Copy link
Collaborator

@machour machour commented Apr 28, 2023

I've been bitten by this in production a while ago, so here's my attempt at fixing it.

Not sure why we have duplicate types definitions in the react package, instead of a re-export from server-runtime.
Also, not sure this is the smartest fix, would definitely accept a better one

Closes: #6210

  • Docs
  • Tests

Testing Strategy:

Before After
Screenshot 2023-04-28 at 10 24 22 AM Screenshot 2023-04-28 at 10 24 01 AM

@changeset-bot
Copy link

changeset-bot bot commented Apr 28, 2023

🦋 Changeset detected

Latest commit: 2f658b7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages
Name Type
@remix-run/react Patch
@remix-run/server-runtime Patch
@remix-run/testing Patch
@remix-run/cloudflare Patch
@remix-run/deno Patch
@remix-run/dev Patch
@remix-run/node Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
create-remix Patch
@remix-run/css-bundle Patch
@remix-run/architect Patch
@remix-run/express Patch
@remix-run/netlify Patch
@remix-run/serve Patch
@remix-run/vercel Patch
remix Patch
@remix-run/eslint-config Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@machour machour linked an issue Apr 28, 2023 that may be closed by this pull request
1 task
@aaronadamsCA
Copy link
Contributor

Note this is a duplicate of #5787.

@machour
Copy link
Collaborator Author

machour commented May 4, 2023

Good catch @aaronadamsCA, thank you!

@kentcdodds
Copy link
Member

I think this PR is actually better than #5787...

@machour
Copy link
Collaborator Author

machour commented May 4, 2023

Reopening for @brophdawg11 consideration.
I think that we both did half the job 😅

@brophdawg11
Copy link
Contributor

yeah I like this one better too I think

@brophdawg11 brophdawg11 self-assigned this May 5, 2023
@brophdawg11
Copy link
Contributor

FWIW this should be a less common occurence now that #6107 is merged - it was a bug that we were still calling meta() for unrendered routes below the error boundary. But this is still needed if a loader throws to it's own boundary and we will still call meta().

@brophdawg11 brophdawg11 merged commit d8bcdc1 into remix-run:dev May 5, 2023
@brophdawg11 brophdawg11 added the feat:links-meta Issues related to links()/meta() label May 5, 2023
@brophdawg11 brophdawg11 removed their assignment May 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-d8bcdc1-20230506 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.16.1-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.16.1 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False type safety with meta v2
5 participants