-
Notifications
You must be signed in to change notification settings - Fork 10
feat: convert xsd:date and xsd:dateTime literals to JavaScript Date objects #123
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
base: main
Are you sure you want to change the base?
Conversation
…e to JavaScript Date objects Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
|
Eventually it might be better to use https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal/Instant There are some existing polyfills, maybe it's better to use them rather than making a breaking change once Temporal becomes more broadly available? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal#see_also |
|
I'd be inclined to stick with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances literal handling by adding automatic conversion of xsd:date and xsd:dateTime RDF literals to JavaScript Date objects, similar to existing numeric and boolean conversions.
Key Changes
- Reading:
xsd:dateandxsd:dateTimeliterals are automatically parsed intoDateobjects instead of remaining as strings - Writing:
Dateobjects can be set on date fields and are serialized appropriately (date-only format forxsd:date, full ISO 8601 forxsd:dateTime) - Types: Updated
LiteralLikeandRawValuetype definitions to includeDate, enabling proper TypeScript type checking
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
packages/jsonld-dataset-proxy/src/util/nodeToJsonldRepresentation.ts |
Adds Date object creation for xsd:date and xsd:dateTime literals during reading |
packages/jsonld-dataset-proxy/src/util/getNodeFromRaw.ts |
Implements Date serialization logic with format selection based on datatype (date-only vs full ISO) |
packages/jsonld-dataset-proxy/src/types.ts |
Extends LiteralLike type to include Date |
packages/jsonld-dataset-proxy/src/util/RawObject.ts |
Extends RawValue type to include Date |
packages/jsonld-dataset-proxy/src/subjectProxy/getValueForKey.ts |
Updates return type signature to include Date |
packages/jsonld-dataset-proxy/test/patientExampleData.ts |
Updates PatientShape type to accept Date for birthdate field |
packages/jsonld-dataset-proxy/test/nodeToJsonRepresentation.test.ts |
Adds comprehensive tests for date/dateTime parsing including timezone handling |
packages/jsonld-dataset-proxy/test/jsonldDatasetProxy.test.ts |
Updates test expectations to validate Date objects and fixes date format in test data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else { | ||
| // Use full ISO format for xsd:dateTime and other date-related types | ||
| return literal(value.toISOString(), datatype); |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The else branch at line 42-44 converts any Date object to ISO string format when the datatype is not xsd:date, even for non-date-related datatypes. This could lead to unexpected behavior if a Date object is accidentally passed for a field with a different datatype (e.g., xsd:string). Consider restricting Date object handling to only xsd:date and xsd:dateTime datatypes, and either throw an error or return undefined for other datatypes.
| } else { | |
| // Use full ISO format for xsd:dateTime and other date-related types | |
| return literal(value.toISOString(), datatype); | |
| } else if (datatype === "http://www.w3.org/2001/XMLSchema#dateTime") { | |
| // Use full ISO format for xsd:dateTime | |
| return literal(value.toISOString(), datatype); | |
| } else { | |
| // For non-date-related datatypes, do not coerce Date values | |
| return undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied this suggestion in 587aa3e. Date objects are now only accepted for xsd:date and xsd:dateTime datatypes, returning undefined for other datatypes.
| case "http://www.w3.org/2001/XMLSchema#dateTime": | ||
| return new Date(literal.value); |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Date constructor may create an invalid Date object if the literal value is malformed. Consider adding validation to check if the date is valid using isNaN(date.getTime()) and returning the original string value if parsing fails, maintaining backward compatibility with invalid date strings.
| case "http://www.w3.org/2001/XMLSchema#dateTime": | |
| return new Date(literal.value); | |
| case "http://www.w3.org/2001/XMLSchema#dateTime": { | |
| const date = new Date(literal.value); | |
| return isNaN(date.getTime()) ? literal.value : date; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied this suggestion in 587aa3e. Invalid dates now return the original string value.
| ).toBe("P1Y2M3D"); | ||
| }); | ||
|
|
||
| it("throws an error when it encoutners a quad that is not a Liter, NamedNode, or BlankNode", () => { |
Copilot
AI
Jan 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error: "encoutners" should be "encounters" and "Liter" should be "Literal".
| it("throws an error when it encoutners a quad that is not a Liter, NamedNode, or BlankNode", () => { | |
| it("throws an error when it encounters a quad that is not a Literal, NamedNode, or BlankNode", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 587aa3e.
|
@copilot apply changes based on the comments in this thread |
- Validate Date parsing and return original string for invalid dates - Restrict Date coercion to only xsd:date and xsd:dateTime datatypes - Add tests for invalid date strings - Add timezone edge case test for xsd:date serialization - Fix spelling errors in test description Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
Applied all the requested changes in commit 587aa3e:
|
|
@copilot please revert the support for invalid dates |
Per reviewer request, reverted the invalid date validation that returned the original string for malformed dates. Invalid dates will now return an Invalid Date object, consistent with standard JavaScript Date behavior. Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
nodeToJsonldRepresentation.tsto convertxsd:dateTimeandxsd:dateliterals to JavaScriptDateobjectsDateinLiteralLikeandRawValuetypesgetNodeFromRaw.tsto handleDateobjects when writing back to the datasetxsd:dateas date-only (YYYY-MM-DD)xsd:dateTimegetValueForKey.tsto includeDatein the return typenodeToJsonRepresentation.test.tsDateobjects for date fieldsDatevalue to a date fieldOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.