-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
fix(gatsby): quick check if string looks like a date #12700
Conversation
Awesome! #12692 (comment) |
I think we should also revert #12533 which increased our supported date formats from 24 to 55! |
@@ -99,8 +99,13 @@ const GraphQLDate = new GraphQLScalarType({ | |||
// Check if this is a date. | |||
// All the allowed ISO 8601 date-time formats used. | |||
function isDate(value) { | |||
// quick check if value does not look like a date | |||
if (typeof value === `number` || !/^\d{4}/.test(value)) { |
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.
I'm not sure an inline regex is the fastest way of determining if the first 4 values are numeric.
You could pre-compile, or alternatively even have a set of possible single digit strings, and iterate over the first 4 and break if it's not in the set.
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.
According to this benchmark, it seems inline regex or pre-compiled is actually fastest.
https://jsperf.com/check-whether-string-starts-with-four-numbers
In Chrome it doesn't appear to matter if it's inline or not, in Safari I notice a 2% improvement in pre-compiling.
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.
I tried both ways and the change was marginal, about 600s to build the schemas. Without the patch, it was 8000s. My guess is that the regex is optimized automatically.
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.
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.
Let's ship it.
Description
Checks if strings start with 4 numbers as all ISO 8601 date-time are using YYYY syntax
Related Issues
#12692