Skip to content

feat: Add new AppField API for Angular #1541

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

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

Conversation

crutchcorn
Copy link
Member

@crutchcorn crutchcorn commented May 30, 2025

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
  • Triple check no breaking changes were made

Copy link

nx-cloud bot commented May 30, 2025

View your CI Pipeline Execution ↗ for commit e3ca99e.

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

☁️ Nx Cloud last updated this comment at 2025-06-19 13:55:49 UTC

@flensrocker
Copy link

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...

@crutchcorn
Copy link
Member Author

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

Copy link

pkg-pr-new bot commented May 31, 2025

More templates

@tanstack/angular-form

npm i https://pkg.pr.new/@tanstack/angular-form@1541

@tanstack/form-core

npm i https://pkg.pr.new/@tanstack/form-core@1541

@tanstack/lit-form

npm i https://pkg.pr.new/@tanstack/lit-form@1541

@tanstack/react-form

npm i https://pkg.pr.new/@tanstack/react-form@1541

@tanstack/solid-form

npm i https://pkg.pr.new/@tanstack/solid-form@1541

@tanstack/svelte-form

npm i https://pkg.pr.new/@tanstack/svelte-form@1541

@tanstack/vue-form

npm i https://pkg.pr.new/@tanstack/vue-form@1541

commit: e3ca99e

@flensrocker
Copy link

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
Copy link

You may want to cherry-pick
flensrocker@ae5751d

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
Copy link

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.

crutchcorn and others added 5 commits June 17, 2025 01:22
Co-authored-by: Lars Hanisch <blog@flensrocker.de>
This PR breaks the abiltiy to run tests thanks to Testing Library seemingly not supporting Angular 20
Copy link

codecov bot commented Jun 19, 2025

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 (0d949d6) to head (e3ca99e).

Files with missing lines Patch % Lines
packages/angular-form/src/tanstack-field.ts 94.28% 2 Missing ⚠️
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              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

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