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.

Sign up for GitHub

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

Jump to bottom

fix(remix-server-runtime): update error message when route doesn't have exports #2779

Merged
merged 2 commits into from
May 17, 2022
Merged

fix(remix-server-runtime): update error message when route doesn't have exports #2779

merged 2 commits into from
May 17, 2022

Conversation

jacob-ebey
Copy link
Member

Closes: #

  • Docs
  • Tests

kentcdodds
kentcdodds approved these changes Apr 13, 2022
View reviewed changes
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Super 👍 Thanks.

kentcdodds
kentcdodds requested changes Apr 13, 2022
View reviewed changes
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Actually, could we make the error message a little smarter? If they make a GET then we know they need a loader or default export, so no reason to say anything about the action. If they make a non-GET then we know they need an action so no reason to say anything about the loader right? And actually the code here is only checking for the existence of a loader. Do we know by this point that there's not an action or default?

@MichaelDeBoey MichaelDeBoey changed the title fix: update error message when route doesn't have exports fix(remix-server-runtime): update error message when route doesn't have exports May 17, 2022
@MichaelDeBoey MichaelDeBoey requested a review from kentcdodds May 17, 2022 10:26
kentcdodds
kentcdodds approved these changes May 17, 2022
View reviewed changes
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Super 👍

@kentcdodds kentcdodds merged commit 89327a9 into dev May 17, 2022
@kentcdodds kentcdodds deleted the jacob/rem-976-error-message-when-a-route-doesnt-export branch May 17, 2022 15:17
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v1.5.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
Copy link
Contributor

🤖 Hello there,

We just published version v1.5.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!

@donavon donavon mentioned this pull request Jul 31, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Aug 1, 2022
Open
@donavon donavon mentioned this pull request Aug 1, 2022
Open
This was referenced Nov 28, 2023
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelDeBoey MichaelDeBoey MichaelDeBoey approved these changes

@kentcdodds kentcdodds kentcdodds approved these changes

@machour machour machour approved these changes

Assignees
No one assigned
Labels
CLA Signed package:server-runtime
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jacob-ebey @machour @kentcdodds @MichaelDeBoey