Skip to content
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-transformer-remark): don't convert Date objects #10924

Merged
merged 2 commits into from
Jan 25, 2019

Conversation

stefanprobst
Copy link
Contributor

gatsby-transformer-remark parses frontmatter with gray-matter and then iterates through frontmatter fields again to transform Dates into strings.

This PR proposes to only allow types from the YAML 1.2 spec, which has no timestamp type anymore (the 1.1 spec has). This way, dates will automatically be parsed as strings.

For the different schema options that the yaml parser accepts see here.

@pieh
Copy link
Contributor

pieh commented Jan 8, 2019

I actually wonder if this is still problem (fact that Date object are potentially used). I remember fixing inconsistencies when Date object was used, then cached in .cache/redux-state.json and read from in next run - it might be just unnecessary code and we don't need to specifically declare schema without that don't support Date objects here?

@stefanprobst
Copy link
Contributor Author

Ah indeed, just checked and it works fine because of isMixOfDateObjectsAndDateStrings.

@stefanprobst stefanprobst changed the title fix(gatsby-transformer-remark): allow YAML 1.2 types only Jan 8, 2019
@freiksenet
Copy link
Contributor

@stefanprobst @pieh Is this PR still relevant?

@wardpeet wardpeet added the status: awaiting author response Additional information has been requested from the author label Jan 25, 2019
@pieh
Copy link
Contributor

pieh commented Jan 25, 2019

Yeah, it's still worth to get in, will just validate that change now so at least 2 sets of eyes checked it.

Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

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

Validated and works as expected. Thanks @stefanprobst!

@pieh pieh merged commit 4463f52 into gatsbyjs:master Jan 25, 2019
@stefanprobst stefanprobst deleted the allow-yaml-1.2-types-only branch January 25, 2019 10:54
@zslabs zslabs mentioned this pull request Feb 7, 2019
@zslabs
Copy link
Contributor

zslabs commented Feb 7, 2019

I have a date field within my project that has some written as strings, and others not (have content creators doing both unfortunately); and this PR appears to be breaking the sorting functionality I'm using.

DSchau added a commit that referenced this pull request Feb 8, 2019
<!--
  Have any questions? Check out the contributing docs at https://gatsby.app/contribute, or
  ask in this Pull Request and a Gatsby maintainer will be happy to help :)
-->

## Description

Quick fix to standardize on date format strings, which appears to have been broken with #10924

## Related Issues

<!--
  Link to the issue that is fixed by this PR (if there is one)
  e.g. Fixes #1234, Addresses #1234, Related to #1234, etc.
-->
gurpreet-hanjra pushed a commit to gurpreet-hanjra/gatsby that referenced this pull request Feb 14, 2019
<!--
  Have any questions? Check out the contributing docs at https://gatsby.app/contribute, or
  ask in this Pull Request and a Gatsby maintainer will be happy to help :)
-->

## Description

Quick fix to standardize on date format strings, which appears to have been broken with gatsbyjs#10924

## Related Issues

<!--
  Link to the issue that is fixed by this PR (if there is one)
  e.g. Fixes gatsbyjs#1234, Addresses gatsbyjs#1234, Related to gatsbyjs#1234, etc.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting author response Additional information has been requested from the author
5 participants