Infer dataType from value set with typed literals #125
Conversation
- Added inferDataTypeFromValueSet helper function to extract datatype from ObjectLiteral values - Updated ShexJContextVisitor to infer @type for predicates with typed literal value sets - Updated ShexJTypingTransformer to generate correct TypeScript types for typed literal values - Added unit tests for inferDataTypeFromValueSet - Added booleanValueSet test case demonstrating the feature Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
- Use Set for tracking added primitive types in TypeScript generation - Add robust isObjectLiteral type guard function - Improve null checks for value detection Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds datatype inference for ShEx value sets containing typed literals (e.g., "true"^^xsd:boolean), fixing an issue where the JSON-LD context and TypeScript types did not properly infer datatypes from such value sets. Previously, this resulted in empty TypeScript types and missing @type annotations in the JSON-LD context.
Key Changes
- Introduced a helper function (
inferDataTypeFromValueSet) to extract common datatypes from value sets where all ObjectLiteral values share the same type - Modified context generation to add
@typeannotations when typed literal value sets are detected - Enhanced TypeScript generation to produce correct primitive types (boolean, number, string) for ObjectLiteral values
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/schema-converter-shex/src/context/util/inferDataTypeFromValueSet.ts |
New helper function that infers a common datatype from value sets with typed ObjectLiterals |
packages/schema-converter-shex/test/inferDataTypeFromValueSet.test.ts |
Unit tests for the new helper function covering various edge cases |
packages/schema-converter-shex/test/testData/booleanValueSet.ts |
Integration test case demonstrating boolean value set inference |
packages/schema-converter-shex/test/testData/testData.ts |
Imports and registers the new boolean value set test case |
packages/schema-converter-shex/src/context/ShexJContextVisitor.ts |
Uses the new helper to add @type to predicates with inferred datatypes |
packages/schema-converter-shex/src/typing/ShexJTypingTransformer.ts |
Refactored datatype-to-TypeScript mapping into reusable datatypeToDomType function; handles ObjectLiteral values in value sets |
packages/schema-converter-shex/src/context/JsonLdContextBuilder.ts |
Minor formatting fixes (semicolon and quotes) |
packages/react/src/methods/useDataset.ts |
Minor formatting fixes (imports and trailing semicolons) |
packages/subscribable-dataset/src/TransactionDatasetFactory.ts |
Minor formatting fix (multi-line import) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (domType && !addedPrimitiveTypes.has(domType)) { | ||
| addedPrimitiveTypes.add(domType); | ||
| valuesUnion.members.push(domType); |
There was a problem hiding this comment.
The logic in the NodeConstraint transformer for handling value sets has a potential issue. When an ObjectLiteral has a type that is recognized by datatypeToDomType, it correctly adds the primitive type to the union. However, if datatypeToDomType returns undefined (for an unrecognized datatype), the code silently ignores that value and doesn't add anything to the union.
This means that if you have a value set with an unrecognized datatype like "value"^^custom:type, nothing will be added to the TypeScript type for that value, which could lead to incomplete type definitions. Consider adding a fallback to treat unrecognized datatypes as strings, similar to the behavior when an ObjectLiteral has no type specified (lines 370-375).
| if (domType && !addedPrimitiveTypes.has(domType)) { | |
| addedPrimitiveTypes.add(domType); | |
| valuesUnion.members.push(domType); | |
| // If datatype is unrecognized, fall back to string (same as no datatype) | |
| const effectiveType = domType ?? dom.type.string; | |
| if (!addedPrimitiveTypes.has(effectiveType)) { | |
| addedPrimitiveTypes.add(effectiveType); | |
| valuesUnion.members.push(effectiveType); |
| // If there's only one type in the union, return that type directly | ||
| if (valuesUnion.members.length === 1) { |
There was a problem hiding this comment.
The code doesn't handle the case where valuesUnion.members is empty. If a value set contains only unrecognized value types (e.g., only IriStemRange, LiteralStemRange, etc. that aren't currently handled), the union will have zero members. This would result in returning an empty union type, which may cause TypeScript errors. Consider adding a check: if valuesUnion.members.length === 0, return a default type like dom.type.undefined or dom.type.string to ensure valid TypeScript is always generated.
| // If there's only one type in the union, return that type directly | |
| if (valuesUnion.members.length === 1) { | |
| // If no recognized values were added, fall back to undefined | |
| if (valuesUnion.members.length === 0) { | |
| return dom.type.undefined; | |
| } else if (valuesUnion.members.length === 1) { | |
| // If there's only one type in the union, return that type directly |
When a ShEx schema contains value sets with typed literals (e.g.,
"true"^^xsd:boolean), the JSON-LD context and TypeScript types were not inferring the datatype. This PR adds datatype inference for such cases.Changes
inferDataTypeFromValueSet.ts): Extracts common datatype from value sets where all ObjectLiteral values share the same typeShexJContextVisitornow adds@typeto predicates with typed literal value setsShexJTypingTransformergenerates correct primitive types (boolean, number, string) for ObjectLiteral values; refactored datatype mapping into reusabledatatypeToDomTypehelperExample
Before:
isEnabled: {}isEnabled: undefinedAfter:
isEnabled: { "@type": "http://www.w3.org/2001/XMLSchema#boolean" }isEnabled: booleanWarning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
www.w3.org/usr/local/bin/node /usr/local/bin/node /home/REDACTED/work/ldo/ldo/node_modules/jest-worker/build/workers/processChild.js k/node_modules/.tsc --project tsconfig.esm.json sh odul�� cjs.js n(dns block)/usr/local/bin/node /usr/local/bin/node /home/REDACTED/work/ldo/ldo/node_modules/jest-worker/build/workers/processChild.js ode sh odul�� cjs.js n /node config.cjs.json node build:cjs bin/node node tgra�� && node ../../scripts/rename-to-cjs.js build:cjs node_modules/@npmcli/run-script/lib/node-gyp-bin/sh d:esm build es/.bin/node node(dns block)/usr/local/bin/node node --experimental-vm-modules test-boolean-valueset.mjs node tgra�� ist && npm run build:ts && npm run copy-files && npm run update-permission build:cjs odules/.bin/sh cjs.js node es/.bin/sh sh -c && npm run build:esm sort es/connected-nextgraph/node_modules/.bin/node -t: -k2,2 es/.bin/sh sh(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
dataTypefrom value set #31💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.