form icon indicating copy to clipboard operation
form copied to clipboard

feat: Add new AppField API for Angular

Open crutchcorn opened this issue 7 months ago • 7 comments

This PR adds in a new AppField API for our Angular adapter, which massively improves the DX of the library.

Huge thank you to @flensrocker for the inspiration and pair-coding on this problem!

TODOs

  • [ ] Write tests
  • [ ] Write docs
  • [x] Triple check no breaking changes were made

crutchcorn avatar May 30 '25 16:05 crutchcorn

View your CI Pipeline Execution ↗ for commit 2bdaf2482fb941dbd9cb5081ad21ead67a090d59

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded 18s View ↗
nx run-many --target=build --exclude=examples/** ✅ Succeeded <1s View ↗

☁️ Nx Cloud last updated this comment at 2025-07-05 10:38:08 UTC

nx-cloud[bot] avatar May 30 '25 16:05 nx-cloud[bot]

The dependency injection doesn't guarantee type safety, so now we are able to assign a number field to the app-text-field.

To be fair currently we don't have typesafety in the "simple" example, too, when assigning the name input of the tanstackField directive (and we don't have a runtime error, just an additional misspelled field in the form data). I don't know if that's a problem with the Angular Template compiler or if we can improve the DX even more.

I wonder if it's possible to create something like the TanStackFieldComponent from this PR but with a restricted type for TData and use that as a AppTextField.

More research is needed...

flensrocker avatar May 31 '25 08:05 flensrocker

The dependency injection doesn't guarantee type safety, so now we are able to assign a number field to the app-text-field.

This is known, but trivially resolved by adding a:

value = input.required<string>();

As a property no the app-text-component, just as suggested in React.

To be fair currently we don't have typesafety in the "simple" example, too, when assigning the name input of the tanstackField directive (and we don't have a runtime error, just an additional misspelled field in the form data). I don't know if that's a problem with the Angular Template compiler or if we can improve the DX even more.

That... Isn't accurate from my testing when I first wrote the adapter of Angular... I wonder if there's been a regression in Angular on this area. Reproduction and deeper investigation would be welcome

crutchcorn avatar May 31 '25 08:05 crutchcorn

I made a few steps back to get a better understanding of what's going on.

In today's timeslot I removed the extra layers and derived the AppTextField directly from TanStackField and narrowed TName down to const TName extends DeepKeysOfType<TParentData, string>, so we only allow names of string fields.

For now with a complete set of all the types (because I don't know which one can be any). But at least I get an error, if I try to pass the name of a number field to AppTextField.

I guess in other frameworks the "glue" to make this friendlier is the fieldContext, withForm etc.

https://github.com/flensrocker/tan-stack-form/blob/cefcb102016e067e83a5de59287c7a2df332a0c3/examples/angular/large-form/src/app/app.component.ts#L30

flensrocker avatar Jun 01 '25 13:06 flensrocker

You may want to cherry-pick https://github.com/flensrocker/tan-stack-form/commit/ae5751d3a5142b67967bac8062fdac45295e0743

The refactoring to input signals of the TanStackField broke the "simple" example, because the api would be created after the template tries to read its name properties etc.

Does the (unnecessary) update of the options on the first run of the effect hurt? Or is the "no change actually happened" intercepted somewhere in the underlying store? Otherwise a "needs update because of some change" check could be implemented inside the effect comparing the next values of the options to the current ones of api.

flensrocker avatar Jun 01 '25 14:06 flensrocker

To be fair currently we don't have typesafety in the "simple" example, too, when assigning the name input of the tanstackField directive (and we don't have a runtime error, just an additional misspelled field in the form data). I don't know if that's a problem with the Angular Template compiler or if we can improve the DX even more.

That... Isn't accurate from my testing when I first wrote the adapter of Angular... I wonder if there's been a regression in Angular on this area. Reproduction and deeper investigation would be welcome

I just tried with the main branch. When I run the "simple" Angular example I can specify whatever I want on the name input of tanstackField. That's weird.

flensrocker avatar Jun 01 '25 15:06 flensrocker

Codecov Report

Attention: Patch coverage is 95.74468% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.36%. Comparing base (a8279d5) to head (2bdaf24). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/angular-form/src/tanstack-field.ts 94.28% 2 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1541      +/-   ##
==========================================
+ Coverage   89.24%   89.36%   +0.12%     
==========================================
  Files          31       33       +2     
  Lines        1432     1458      +26     
  Branches      366      368       +2     
==========================================
+ Hits         1278     1303      +25     
- Misses        137      138       +1     
  Partials       17       17              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Jun 19 '25 13:06 codecov[bot]

Thank you, that was fun!

flensrocker avatar Jul 05 '25 10:07 flensrocker