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

feat(action): Error message for when calling raw action #402

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gabrielmfern
Copy link

A few times I've called the actions and taking a bit to figure out
this was the case, so I decided to add an error message that might
be helpful to others in this. The error message currently looks like:

Seems like you are directly calling a function wrapped in "action". To properly use an action, you will need to first call "useAction". 

So, if you have code like this:

1    const myFunc = action(...);
2  
3 /  function MyComponent() {
4 |    return <button 
5 |      onClick={() => myFunc(...)}
  |____________________^ This is where the error is going to happen
6      >
7        Click me!
8      </button>
9    }

You will need to change it to something like:

1    const myAction = action(...);
2    
3    function MyComponent() {
4      const callMyAction = useAction(myAction);
5    
6      return <button 
7        onClick={() => callMyAction(...)}
8      >
9        Click me!
10     </button>
11   }

This is the case because the action will need to tune itself to the surrounding context for the Router so that it can
keep track of all the submissions. See https://docs.solidjs.com/solid-router/concepts/actions#creating-actions for more information on how to use actions.

Tried writing it to be clear and show examples so that the person can understand
even if they don't end up look into the docs for this.

Examples of how the error looks like on different browsers:
Firefox Chrome

Let me know if the condition it checks for makes sense in this situation.

Thanks!

Copy link

changeset-bot bot commented Apr 13, 2024

🦋 Changeset detected

Latest commit: 0504bbb

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

This PR includes changesets to release 1 package
Name Type
@solidjs/router 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

@ryansolid
Copy link
Member

ryansolid commented Apr 29, 2024

Hmm.. that's a lot of text..Enough to impact the bundle size. We need to filter that out of prod via a dev check.

@gabrielmfern
Copy link
Author

Hey @ryansolid, thanks for the feedback.

We need to filter that out of prod via a dev check.

Would a check on isDev be enough here (like bellow), or is there a conventional way? Haven't looked around the code very much to know.

+if (isDev) {
   if (typeof this !== "object" || !Object.hasOwn(this, "r")) {
     throw new Error(`Seems like you are directly calling a function wrapped in "action". To properly use an action, you will need to first call "useAction". 

Also, what parts do you think could be removed to make it more concise? Maybe the parts meant to be code blocks? I suppose they aren't exactly needed, just are good for illustration purposes.

@gabrielmfern gabrielmfern force-pushed the feat/error-message-for-raw-call-to-action branch from 7e43b54 to 0504bbb Compare April 29, 2024 22:57
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.

2 participants