solidjs / solid-router

A universal router for Solid inspired by Ember and React Router
MIT License
1.11k stars 138 forks source link

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

Open gabrielmfern opened 3 months ago

gabrielmfern commented 3 months ago

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!

changeset-bot[bot] commented 3 months ago

🦋 Changeset detected

Latest commit: 0504bbbfd7c247cd42b5f03f0e8d9447795bc9a7

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 commented 3 months ago

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 commented 3 months ago

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.