Skip to content

Union UISchemaElement type #2436

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

Merged
merged 5 commits into from
Apr 8, 2025

Conversation

IceFreez3r
Copy link
Contributor

Changes

as discussed in #2109

  • Renamed UISchemaElement to BaseUISchemaElement

  • Added new UISchemaElement which is a union of all uischema options, including BaseUISchemaElement for backwards-compatibility

  • Same treatment for conditions

    • renamed Condition to BaseCondition
    • added Condition as new union type
  • Unrelated to the main changes, I found that .nvmrc had a different node version than required in the README, so I updated it to node 22

Install node.js (only Node v22+ < 23 is currently supported)

Compliance

Build and lint both run through. I have one failing test: test/testers.test.ts, but I checked and it also fails without my changes, so I assume it's unrelated. Error message for reference:

$ nvm use
Now using node v22.14.0 (npm v10.9.2)
$ pnpm test
> jsonforms-monorepo@ test /path/to/jsonforms
> lerna run test

lerna notice cli v6.6.2


    ✖  @jsonforms/core:test
       > @jsonforms/[email protected] test /path/to/jsonforms/packages/core
       > ava
       
       
         Uncaught exception in test/testers.test.ts
       
         Error: Cannot find module '/path/to/jsonforms/packages/core/node_modules/.cache/ava/import-from-project.mjs' imported from /path/to/jsonforms/node_modules/.pnpm/[email protected][email protected]/node_modules/ava/lib/worker/base.js
       
         Error: Cannot find module '/path/to/jsonforms/packages/core/node_modules/.cache/ava/import-from-project.mjs' imported from /path/to/jsonforms/node_modules/.pnpm/[email protected][email protected]/node_modules/ava/lib/worker/base.js
             at finalizeResolution (node:internal/modules/esm/resolve:275:11)
             at moduleResolve (node:internal/modules/esm/resolve:860:10)
             at defaultResolve (node:internal/modules/esm/resolve:984:11)
             at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:685:12)
             at ModuleLoader.#cachedDefaultResolve (node:internal/modules/esm/loader:634:25)
             at ModuleLoader.resolve (node:internal/modules/esm/loader:617:38)
             at ModuleLoader.getModuleJobForImport (node:internal/modules/esm/loader:273:38)
             at onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:577:36)
             at TracingChannel.tracePromise (node:diagnostics_channel:344:14)
             at ModuleLoader.import (node:internal/modules/esm/loader:576:21)
       
         ✘ test/testers.test.ts exited with a non-zero exit code: 1

@CLAassistant
Copy link

CLAassistant commented Apr 7, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

netlify bot commented Apr 7, 2025

Deploy Preview for jsonforms-examples ready!

Name Link
🔨 Latest commit 9e0d982
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/67f525b679b26d0008e53020
😎 Deploy Preview https://deploy-preview-2436--jsonforms-examples.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@coveralls
Copy link

coveralls commented Apr 8, 2025

Coverage Status

coverage: 82.517%. remained the same
when pulling 9e0d982 on IceFreez3r:uischema-types
into 9acd631 on eclipsesource:master.

Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

Hi @IceFreez3r , thanks for the contribution ❤️
The changes already look quite good to me. I have only two small asks:

  • Please add a comment to the new Condition and UISchemaElement types as they are public API
  • Please add a new section for the 3.6 (which is the next one) release and explain the type changes to the migration guide.

@IceFreez3r
Copy link
Contributor Author

@lucas-koehler Added the requested changes. Is the migration section like you had in mind?

Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

@IceFreez3r LGTM now! Thanks for the updates :)

@lucas-koehler lucas-koehler linked an issue Apr 8, 2025 that may be closed by this pull request
@lucas-koehler lucas-koehler merged commit ab09d94 into eclipsesource:master Apr 8, 2025
8 checks passed
@IceFreez3r IceFreez3r deleted the uischema-types branch April 8, 2025 15:28
* This includes all layout elements, control elements, label elements,
* group elements, category elements and categorization elements.
*/
export type UISchemaElement =
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this is nice to see! I created this same thing in jsonforms-antd-renderers, but with a type argument for the literal type of the corresponding jsonschema. Having the schema as a type argument allows us to intersect UISchemaElement with SchemaAwareScope, which traverses the jsonschema (via the type system, not at runtime), to give us autocomplete and type-checking when defining UISchemas, ie for valid scope values, or options configurations.

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.

Type error for UISchema object
5 participants