Repository navigation
Conversation
Code Metrics Report
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%)
Reported by octocov |
interacsean
left a comment
There was a problem hiding this comment.
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.
|
@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>
Resolved and pushed in merge commit |
|
Reverted the AppShell peer dependency added in 41502f5 via a57c307 after checking the existing ERP application template. Why the peer dependency does not solve compatibilityThe 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
For this change, older AppShell metadata types reject 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 packageWe also considered exposing the generator through 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. |
|
Following up on the metadata compatibility discussion, I would like to propose build-time diagnostics through the Vite plugin. MotivationFrontend type-checking can reject unsupported metadata such as The reverse situation is harder to detect: older generated metadata still represents Decimal fields as We should make these situations diagnosable without requiring AppShell in the backend or emitting warnings in the customer-facing browser. Proposed approach
Diagnostics would not modify the browser code, display a Toast, or fail the build. Compatibility rules and limitationsCompatibility 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 |
|
@copilot resolve the merge conflicts in this pull request |
# Conflicts: # docs-manifest.json Co-authored-by: IzumiSy <982850+IzumiSy@users.noreply.github.com>
Resolved and pushed in merge commit |
Motivation
TailorDB decimal values are returned by GraphQL as strings to preserve precision, but the SDK plugin previously generated
numbermetadata 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
decimalmetadata 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 existingnumberfilter 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