graphql-iso-date
graphql-iso-date copied to clipboard
Strip time component of parsed Date
Fixes #106.
This PR strips the time component from a date string when being parsed as a GraphQLDate to allow services both using GraphQLDate fields to delegate to each other.
Codecov Report
Merging #107 into master will not change coverage. The diff coverage is
100%.
@@ Coverage Diff @@
## master #107 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 5 5
Lines 146 148 +2
Branches 40 40
=====================================
+ Hits 146 148 +2
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/date/index.js | 100% <100%> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 8979ce8...8218033. Read the comment docs.
If I'm reading this correctly, won't this still deserialize dates as a Date object, which will then get stringified to full datetime strings?
We ran into this issue, too, and we solved it by wrapping the return values in custom Date class that looks like
class DateOnly extends Date {
toISOString() {
return super.toISOString().slice(0, 10);
}
}
They get stringified to full datetime strings but with a zero-ed time portion. This change makes the ingress more flexible to allow full datetime strings, but ignoring the time portion.
This would be great to get merged in, as I'm having the same problem referenced in #106
edit: This approach doesn't work. If you're looking for a solution, use @sloria 's example below.
I think @sloria is on to something. I ended up taking his code and creating a custom scalar that decorates the GraphQLDate class with his custom Date class:
import { GraphQLScalarType } from "graphql";
import { GraphQLDate } from "graphql-iso-date";
// Here we're overriding the parsing behavior in GraphQLDate to provide
// some better parsing of Dates
// see https://github.com/excitement-engineer/graphql-iso-date/issues/106
export default class GraphQLDateOnly extends GraphQLScalarType {
constructor() {
super({
name: "Date",
serialize(value) {
return GraphQLDate.serialize(value);
},
parseValue(value) {
const date = GraphQLDate.parseValue(value);
return new DateOnly(date);
},
parseLiteral(ast) {
const date = GraphQLDate.parseLiteral(ast);
return new DateOnly(date);
}
});
}
}
class DateOnly extends Date {
toISOString() {
console.log("Parsing date");
return super.toISOString().slice(0, 10);
}
}
Yes, we did something similar:
/* @flow */
import { GraphQLScalarType } from 'graphql';
import { GraphQLDate as BaseGraphQLDate } from 'graphql-iso-date';
class DateOnly extends Date {
toISOString() {
return super.toISOString().slice(0, 10);
}
}
const {
parseValue: baseParseValue,
parseLiteral: baseParseLiteral,
...config
} = BaseGraphQLDate.toConfig();
export default new GraphQLScalarType({
...config,
parseValue: value => new DateOnly((baseParseValue(value): any)),
parseLiteral: ast => new DateOnly((baseParseLiteral(ast): any)),
});
Can we please merge this in? My team is encountering the same issue, and we are implementing our own version of this in our code as a workaround.
I'm still not sure this PR does the correct thing. I think using DateOnly that stringifies to a date (not datetime) is a more correct approach. Thoughts @MarkPolivchuk ?
How would you go about converting 2017-01-07T00:00:00+01:20 (which is equal to 2017-01-06T22:40:00Z)? Should this be converted to 2017-01-07 or 2017-01-06? There is no clear definition as to what the actual date is given that it is dependent on the time offset. I want to avoid these kind of ambiguities in this library and keep in line with the RFC 3339 specification.