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

Jump to bottom

Fix bug with ?_data request on non-matching route #6820

Merged
merged 2 commits into from
Jul 12, 2023
Merged

Fix bug with ?_data request on non-matching route #6820

merged 2 commits into from
Jul 12, 2023

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Jul 12, 2023 •
edited
Loading

Closes #5738

@changeset-bot
Copy link

changeset-bot bot commented Jul 12, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 0fce002

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/server-runtime Patch
@remix-run/cloudflare Patch
@remix-run/deno Patch
@remix-run/dev Patch
@remix-run/node Patch
@remix-run/react Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
create-remix Patch
@remix-run/architect Patch
@remix-run/express Patch
@remix-run/netlify Patch
@remix-run/serve Patch
@remix-run/testing Patch
@remix-run/vercel Patch
remix Patch
@remix-run/css-bundle 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

@brophdawg11 brophdawg11 mentioned this pull request Jul 12, 2023
Closed
brophdawg11
brophdawg11 commented Jul 12, 2023
View reviewed changes
packages/remix-server-runtime/server.ts
Comment on lines -85 to +87
let match = matches!.find((match) => match.route.id == routeId)!;
response = await build.entry.module.handleDataRequest(response, {
context: loadContext,
params: match ? match.params : {},
params: matches?.find((m) => m.route.id == routeId)?.params || {},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When handleDataRequest was defined and a request came in like /not-a-route?_data=routes/not-a-route we got a valid 404 from the static handler but then errored out here trying to get the params when no matches exist.

@jacob-ebey jacob-ebey merged commit 68877d1 into dev Jul 12, 2023
@jacob-ebey jacob-ebey deleted the brophdawg11/404-data-route branch July 12, 2023 20:45
@github-actions github-actions bot added the awaiting release This issue has been fixed and will be released soon label Jul 12, 2023
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-cdb1d32-20230713 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 github-actions bot mentioned this pull request Jul 13, 2023
Merged
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.19.0-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 github-actions bot mentioned this pull request Jul 19, 2023
Merged
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.19.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 github-actions bot removed the awaiting release This issue has been fixed and will be released soon label Jul 19, 2023
@hi-ogawa hi-ogawa mentioned this pull request Jul 24, 2023
Closed
2 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@jacob-ebey jacob-ebey jacob-ebey approved these changes

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@brophdawg11 @jacob-ebey @ledniy