Skip to content

fix(data-table): preserve string-backed decimal filter values - #582

Open
IzumiSy wants to merge 8 commits into
mainfrom
fix/decimal-field-type
Open

IzumiSy wants to merge 8 commits into
mainfrom
fix/decimal-field-type

Conversation

@IzumiSy

@IzumiSy IzumiSy commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Motivation

TailorDB decimal values are returned by GraphQL as strings to preserve precision, but the SDK plugin previously generated number metadata for them. This made metadata-derived query variables incompatible with string-based Decimal filter inputs. DataTable's numeric filter editors also converted values to JavaScript numbers, rounding high-precision values before sending them to the backend.

Related to https://github.com/tailor-inc/platform-planning/issues/1520.

Design Decision

Preserve the scalar representation

Introduce decimal metadata and filter configuration with numeric operators and string-valued query inputs. A distinct runtime filter type is necessary because editors only receive column configuration: mapping decimals to the existing number filter would retain its numeric coercion and precision loss.

Reuse the numeric filtering flow

Reuse the existing numeric editors and backend ordering, while keeping decimal scalar and range values as strings. Validate decimal range ordering exactly without a new arithmetic dependency. Existing integer and float behavior stays unchanged; membership operators remain programmatic rather than adding new editor UI.

Summary

  • Generate decimal metadata, including decimal array item types, and infer string-backed numeric query inputs and sortable fields.
  • Preserve precision across filter addition, editing, operator transitions, cell context menus, and URL persistence, with regression coverage.
  • Update DataTable documentation and add patch changesets for core and the SDK plugin. Existing consumers should regenerate their metadata after updating the plugin.

@IzumiSy
IzumiSy marked this pull request as ready for review October 6, 2026 07:49
@IzumiSy
IzumiSy requested a review from a team as a code owner October 6, 2026 07:49
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Code Metrics Report

main (95bc8f2) #582 (53e0703) +/-
Coverage 88.7% 88.8% +0.1%
Test Execution Time 2m21s 2m23s +2s
Details
  |                     | main (95bc8f2) | #582 (53e0703) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          88.7% |          88.8% | +0.1% |
  |   Files             |            208 |            209 |    +1 |
  |   Lines             |           6285 |           6297 |   +12 |
+ |   Covered           |           5575 |           5594 |   +19 |
- | Test Execution Time |          2m21s |          2m23s |   +2s |

Code coverage of files in pull request scope (87.9% → 88.5%, patch 97.5%)

Files Coverage +/- Patch Coverage Status
packages/core/src/components/data-table/data-table.tsx 92.1% +0.0% 100.0% modified
packages/core/src/components/data-table/filter-value-utils.ts 100.0% +100.0% 100.0% added
packages/core/src/components/data-table/toolbar.tsx 83.9% +1.5% 94.7% modified
packages/core/src/components/date-field/date-input-group.tsx 84.4% -1.0% - affected
packages/core/src/components/date-field/use-date-field-state.ts 96.8% -0.8% - affected
packages/core/src/types/collection.ts 95.2% +0.2% 100.0% modified

Reported by octocov

@interacsean interacsean left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the changes look good by themselves

While we maintain the sdk-plugin and app-shell together, with commits going in atomically, they are distributed separately. Is there any way to ensure peer-dependency versions. The concern here is somebody updates the sdk-plugin version, which then starts giving "type: decimal", which an older (current) version of app-shell (<= 1.16.0) would not know how to deal with

Otherwise do we need to make support for this new type opt-in on the sdk-side as a configuration option, defaulting to false

Claude says:

The fix is a review comment asking for one of these:

a peer dependency of @tailor-platform/app-shell: ">=" on the plugin
at minimum, a changeset note saying to upgrade both packages together

Existing apps that use db.decimal() today

They currently have type: "number" metadata. What changes depends on what they upgrade:

They upgrade… Result
app-shell only Nothing changes. The metadata still says number, so you get the old behaviour, including rounding. Safe.
Both packages, then regenerate Decimals become strings everywhere. This is the intended fix.
Plugin only, then regenerate The breakage described above.

@IzumiSy

IzumiSy commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

# Conflicts:
#	docs-manifest.json
#	docs-src/components/data-table.docs.outline.md
#	docs/components/data-table.md
#	packages/core/src/components/data-table/data-table.test.tsx
#	packages/core/src/components/data-table/data-table.tsx
#	packages/core/src/components/data-table/filter-value-utils.ts
#	packages/core/src/components/data-table/toolbar.tsx
#	packages/core/src/hooks/use-collection-variables.test.ts

Co-authored-by: IzumiSy <982850+IzumiSy@users.noreply.github.com>

Copilot AI commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved and pushed in merge commit 3752598. Preserved decimal precision support alongside main’s timezone-aware datetime changes. All 399 targeted tests passed, along with type-check, lint, build, declaration, formatting, and documentation checks. CodeQL found no alerts; final review approved.

@IzumiSy

IzumiSy commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Reverted the AppShell peer dependency added in 41502f5 via a57c307 after checking the existing ERP application template.

Why the peer dependency does not solve compatibility

The template installs the SDK plugin in the backend and AppShell in the frontend. The backend generates metadata directly into the frontend.

An AppShell peer dependency on the generator constrains the generator’s installation context, not the frontend consuming its output. It could be satisfied by installing a newer AppShell in the backend while the frontend remains on an incompatible version. The plugin also has no runtime dependency on AppShell; its AppShell imports are type-only.

Decision for this PR

  • Keep the Decimal metadata and filtering fixes.
  • Remove the AppShell peer requirement and the same-dependency-graph restriction.
  • Keep the metadata regeneration guidance in the changeset.

For this change, older AppShell metadata types reject "decimal" when the generated metadata is passed to the typed APIs during frontend type-checking.

If type-checking is bypassed, the existing inference path does not throw for the unknown type: it omits the column’s inferred sort/filter configuration. Metadata-aware URL restoration also drops those sort/filter conditions. We accept that behavior for this change, so no runtime compatibility guard or metadata schema version is being added.

Why keep the generator as a separate package

We also considered exposing the generator through app-shell/sdk-plugin, but I currently prefer keeping it as a separate package.

A backend that otherwise only needs the SDK would have to install the full AppShell UI package, including its UI dependencies and React peer requirements, just to generate metadata. That coupling does not seem justified.

A subpath would also not prevent backend/frontend version skew: the two applications could still install different AppShell versions unless those versions are managed together separately.

Keeping the generator independent better matches the existing backend/frontend separation. Generated metadata compatibility should be checked on the consuming side rather than enforced through a UI dependency on the backend.

@IzumiSy

IzumiSy commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Following up on the metadata compatibility discussion, I would like to propose build-time diagnostics through the Vite plugin.

Motivation

Frontend type-checking can reject unsupported metadata such as "decimal", but the resulting error does not necessarily explain that updating AppShell resolves the issue.

The reverse situation is harder to detect: older generated metadata still represents Decimal fields as "number", so it can pass type-checking while silently missing the intended Decimal behavior.

We should make these situations diagnosable without requiring AppShell in the backend or emitting warnings in the customer-facing browser.

Proposed approach

  1. Add a metadataGeneratorVersion export to the generated file, derived directly from the SDK plugin’s package version.

    • Use the generator package version rather than introducing a separate schema version.
    • Preserve the existing tableMetadata, tableNames, and TableName exports.
    • Do not add an AppShell import to the generated file.
  2. Extend the Vite plugin to inspect generated metadata modules through an early transform hook.

    • Identify generated modules using the existing header: // This file is auto-generated by app-shell-metadata plugin.
    • Read the version statically, without importing or executing the generated module.
    • This avoids depending on a fixed output filename or scanning the entire project.
  3. Emit actionable warnings in the Vite terminal and CI logs.

    • Known outdated metadata: explain that the SDK plugin should be updated and tailor generate rerun.
    • Metadata requiring a newer AppShell: explain that the consuming frontend’s AppShell should be updated.
    • Missing or unrecognized version information: report that compatibility cannot be verified, rather than asserting incompatibility.

Diagnostics would not modify the browser code, display a Toast, or fail the build.

Compatibility rules and limitations

Compatibility should be evaluated against the AppShell version actually resolved by the frontend. Package versions do not need to match, and a version difference alone is not evidence of incompatibility. The supported generator-version ranges would need an explicit, maintained policy.

The check would cover generated modules processed by Vite. In development, a lazy-loaded module may not be checked until it is requested.

It would not run during standalone tsc execution. If CI stops on a type error before running Vite, these diagnostics would not appear, so this would complement—not replace—frontend type-checking.

@IzumiSy

IzumiSy commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

# Conflicts:
#	docs-manifest.json

Co-authored-by: IzumiSy <982850+IzumiSy@users.noreply.github.com>

Copilot AI commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved and pushed in merge commit dda77f8. Regenerated the conflicting documentation manifest, preserving the Decimal fixes and peer-dependency revert alongside latest main. All 366 targeted tests, type-check, lint, formatting, and docs checks passed; CodeQL found no alerts.

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.

3 participants