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

Initial version of Lit adapter for Tanstack form #577

Merged
merged 51 commits into from
Mar 13, 2024

Conversation

Christian24
Copy link
Contributor

@Christian24 Christian24 commented Jan 22, 2024

Builds on top of #588, please merge that first

This closes #498.

This is the first cut.

It has tests, a basic example and the base-lib, there's also Material Web Components example. I added a little QuickStart guide.

Open points:

  • I am not 100% sure this is entirely type safe and I am not sure why it is not. It seems the field options passed .field() can be pretty much anything. Upon further investigation, this seems to be an issue with using an array, so maybe DeepKeys or its use is broken? @crutchcorn?

Copy link

codesandbox-ci bot commented Jan 22, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2024

Codecov Report

Attention: Patch coverage is 67.90123% with 26 lines in your changes are missing coverage. Please review.

Project coverage is 88.84%. Comparing base (c740617) to head (d71df41).

Files Patch % Lines
packages/lit-form/src/tests/simple.ts 56.52% 19 Missing and 1 partial ⚠️
packages/lit-form/src/tanstack-form-controller.ts 82.85% 5 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@                       Coverage Diff                       @@
##           array-and-provider-refactor     #577      +/-   ##
===============================================================
- Coverage                        91.18%   88.84%   -2.34%     
===============================================================
  Files                               26       28       +2     
  Lines                              726      807      +81     
  Branches                           172      185      +13     
===============================================================
+ Hits                               662      717      +55     
- Misses                              59       83      +24     
- Partials                             5        7       +2     

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

@crutchcorn
Copy link
Member

@Christian24 I just realized that we don't have a Lit adapter for TanStack Store. Is there any way we could build this and then utilize it inside of Form's Lit adapter?

That would fall more in line with other stuff

Copy link
Contributor

@fulopkovacs fulopkovacs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't finished reviewing the PR, but I'm really happy that it exists! ☺️

According the the awesome-lit list, the Lit ecosystem doesn't have a lot libraries for forms (I saw only one, but it was last published 2 years ago and has 55 weekly downloads on npm: @vaading/form), so I think the community will find TanStack/form very useful.

Thank you for your work! ☺️

docs/framework/lit/quick-start.md Outdated Show resolved Hide resolved
packages/lit-form/src/tanstack-form-controller.ts Outdated Show resolved Hide resolved
docs/framework/lit/quick-start.md Outdated Show resolved Hide resolved
Christian24 and others added 3 commits February 14, 2024 19:44
Co-authored-by: fuko <43729152+fulopkovacs@users.noreply.github.com>
Co-authored-by: fuko <43729152+fulopkovacs@users.noreply.github.com>
Co-authored-by: fuko <43729152+fulopkovacs@users.noreply.github.com>
@Christian24
Copy link
Contributor Author

@Christian24 I just realized that we don't have a Lit adapter for TanStack Store. Is there any way we could build this and then utilize it inside of Form's Lit adapter?

That would fall more in line with other stuff

I am not sure where it would be needed honestly. Where exactly are you thinking about?

Copy link
Contributor

@fulopkovacs fulopkovacs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'm done with reviewing the whole PR. I have never used Lit, but I tested the examples and read some parts of the Lit docs to get a better understanding on how Lit handles component states (ReactiveController API).

Based on my limited knowledge, this PR is a good first implementation of a Lit adapter for TanStack form. I added a few comments, but they are small ones. ☺️

That said, I should not approve the PR until the conversations started by @crutchcorn on the bind directive and the Lit adapter for tanstack/store have been resolved.

examples/lit/simple/src/index.ts Outdated Show resolved Hide resolved
packages/lit-form/src/control-value-accessor.ts Outdated Show resolved Hide resolved
packages/lit-form/src/control-value-accessor.ts Outdated Show resolved Hide resolved
Fix issue in ControlValueAccessor
Copy link

nx-cloud bot commented Mar 9, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit a561827. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@crutchcorn crutchcorn changed the base branch from main to array-and-provider-refactor March 9, 2024 13:53
Copy link
Contributor

@fulopkovacs fulopkovacs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's only 1 thing that needs to be changed in docs/config.json, because the schema of the config.json files in the TanStack projects have been updated today. Other than that, it's all good.

docs/config.json Outdated Show resolved Hide resolved
@crutchcorn crutchcorn deleted the branch TanStack:main March 13, 2024 11:35
@crutchcorn crutchcorn closed this Mar 13, 2024
@crutchcorn
Copy link
Member

crutchcorn commented Mar 13, 2024

I did not close this PR or delete the upstream branch. Instead, I merged this PR:

#588

Which this PR was supposed to be based off of.

I don't know what's happened - I've pinged GH (thru private channels). In the meantime, I have a backup somewhere, luckily

EDIT: I figured out what happened and have fixed the issue manually. GH has been notified.

@crutchcorn crutchcorn reopened this Mar 13, 2024
@crutchcorn crutchcorn changed the base branch from array-and-provider-refactor to main March 13, 2024 11:53
# Conflicts:
#	docs/config.json
#	examples/react/array/package.json
#	examples/solid/array/package.json
#	examples/vue/array/package.json
#	pnpm-lock.yaml
@crutchcorn crutchcorn requested a review from fulopkovacs March 13, 2024 11:58
@crutchcorn crutchcorn merged commit 565d9d6 into TanStack:main Mar 13, 2024
2 checks passed
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.

5 participants