fabian-hiller / modular-forms

The modular and type-safe form library for SolidJS, Qwik and Preact
https://modularforms.dev
MIT License
1.05k stars 55 forks source link

QwikJS Error with ModularForms: Issue Using 'transform' Property and 'toCustom$' Function #135

Open devcaeg opened 1 year ago

devcaeg commented 1 year ago

Hello @fabian-hiller,

I'm using version v0.20.2 of ModularForms along with version v1.2.11 of QwikJS. I encounter an error from QwikJS when trying to use the "transform" property of a "field" in ModularForms, combined with the "toCustom$" function. The error message displayed in the browser by QwikJS is: "Optimizer should replace all usages of $() with some special syntax. If you need to create a QRL manually, use inlinedQrl() instead."

fabian-hiller commented 1 year ago

Thank you for the hint. I will have a look at this at the end of the week.

devcaeg commented 1 year ago

Hello, I apologize for the inconvenience. Is there an alternative while the error persists? Due to this issue, I can't use the "transform" function on fields. For instance, I need to transform an input field so it only contains numbers and another one so it doesn't have spaces.

fabian-hiller commented 1 year ago

Yes, a workaround could be to use useTask$ to track the change of a field to perform a transformation. However, you must avoid programming an infinite loop. Depending on the use case, Valibot's or Zod's transformation function could also be a workaround.

fabian-hiller commented 1 year ago

Sorry, I have a lot of work at the moment and therefore only come to Modular Forms from time to time. 😕

devcaeg commented 1 year ago

I understand that you might be busy and perhaps don't have enough time, which is completely understandable. Do you plan to continue developing ModularForms, or will the project be abandoned? If you decide to pursue the project in your spare time, would you be open to me creating a Pull Request to fix the "toCustom$" error?

fabian-hiller commented 1 year ago

I plan to continue working on Modular Forms. With the decisions of many frameworks to move to Signals, I can see Modular Forms becoming a relevant open source project in the future. I finished my undergraduate degree three weeks ago with the submission of my bachelor's thesis, and two days after that I moved to New York for my master's degree. These circumstances have meant that I have had less time for Modular Forms in the last three months. As soon as I get more familiar with my new environment, I should have more time for Modular Forms again.

fabian-hiller commented 1 year ago

If you are able to fix the problem, feel free to create a PR or provide me with information. I am interested in getting this fixed soon.

devcaeg commented 1 year ago

Congratulations on your academic achievements! Regarding ModularForms, I'm convinced it will gain a lot of traction in the coming months. It might not have had the initial momentum that Vailbot had, but I have a feeling it could surpass it in a few months, especially in terms of GitHub stars and downloads.

About the "tuCustom$" error, I've been attempting to resolve it today, but I haven't found the solution yet. However, I'll continue to look for a solution. As soon as I have more information on the error or manage to solve it, I'll let you know.

This weekend, I plan to become a small ModularForms sponsor via GitHub, although I'll do it from another GitHub account.

Greetings! 😄

fabian-hiller commented 1 year ago

I have looked at the problem and you are right. I am experiencing the same problem. Since it worked back when I implemented it, I suspect a bug on Qwik's side. The implementation of toCustom$ looks ok so far. Have you already created an issue at Qwik's repository?

devcaeg commented 1 year ago

I opened the problem on QwikJS Discord, but I got no response. Someone tagged you in the question haha.

Today I will open the problem in the QwikJS repository on Github.

fabian-hiller commented 1 year ago

Please link this issue. I noticed that the following does not work either:

transform={toCustomQrl(
  $((value) => value?.toUpperCase()),
  { on: 'change' }
)}

Have you already created a minimal reproduction on StackBlitz without Modular Forms?

devcaeg commented 1 year ago

I have created a reproduction in StackBlitz using ModularForms and another without using ModularForms.

Playback with ModularForms: https://stackblitz.com/edit/qwik-starter-hxqulw?file=src%2Froutes%2Findex.tsx Playback without ModularForms: https://stackblitz.com/edit/qwik-starter-juwwvd?file=src%2Froutes%2Findex.tsx

PS: The error in StackBlitz is a bit different in both cases. In local (development mode) the error appears in the console and in the browser screen. But in StackBlitz the error appears in the console (with another message) but does not start the project.

wmertens commented 1 year ago

@fabian-hiller not related, but why are zod and valibot externals but not peer deps? For the rest the vite.conf and package.json look correct :thinking:

fabian-hiller commented 1 year ago

@fabian-hiller not related, but why are zod and valibot externals but not peer deps? For the rest the vite.conf and package.json look correct 🤔

This is because Modular Forms currently only uses types from both packages, but not logic. This means that Modular Forms does not import JavaScript code from Zod and Valibot. However, your question is valid.

I haven't added it as a dependency yet, because Modular Forms is advertised as dependency-free and it would also increase the bundle size on tools like Bundlephobia extremely. But in the end this is just "marketing".

It also makes no sense as a peer dependency, since we would be forcing our users to install Zod even if they only use Valibot. An alternative would be to release custom npm packages for the adapters. Similar to what React Hook Form does with the resolvers.

wmertens commented 9 months ago

This is a bug in the optimizer, when you have $() in an attribute not ending with $, it will be ignored.

The workaround is to do the $() outside of the JSX template, or to change the prop name to one ending in $.

brandonpittman commented 9 months ago

Yup. Defining the transform function outside the template works. It's the inline example that doesn't work.

devcaeg commented 9 months ago

Oh, thanks haha