-
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-transformer-remark): restore behavior of serializing date-like fields to string #11716
fix(gatsby-transformer-remark): restore behavior of serializing date-like fields to string #11716
Conversation
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.
Validated fixed with https://github.com/DSchau/gatsby-11368
let node | ||
let actions | ||
let createNodeId | ||
beforeEach(() => { |
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.
Some light refactoring here to make this easier to use in the future
We're good to go! |
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.
LGTM! One small question, but don't let it block you from merging.
}) | ||
}) | ||
|
||
describe(`date formatting`, () => { |
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.
Is it worth mentioning here (maybe in comment) that this is only needed because there is bug in gatsby core that doesn't really allow sorting mix of date object and strings?
@@ -23,9 +26,13 @@ describe(`Process markdown content correctly`, () => { | |||
}, | |||
} | |||
|
|||
// Make some fake functions its expecting. | |||
const loadNodeContent = node => Promise.resolve(node.content) | |||
actions = { createNode: jest.fn(), createParentChildLink: jest.fn() } |
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.
can we do:
let markdownNode
beforeEach(() => { markdownNode = null})
createNode: jest.fn(markdownNode => createNode = node)
to avoid those returns in source? return {}
is very confusing to read (without looking at tests)
to fix issues with sort fields ref. * gatsbyjs/gatsby#11368 * gatsbyjs/gatsby#11716
to fix issues with sort fields ref. * gatsbyjs/gatsby#11368 * gatsbyjs/gatsby#11716
…like fields to string (gatsbyjs#11716) * chore: commit * chore: get tests failing (intentionally) to then fix * chore: de-dry * chore: finish tests * chore: revert relative import
…like fields to string (gatsbyjs#11716) * chore: commit * chore: get tests failing (intentionally) to then fix * chore: de-dry * chore: finish tests * chore: revert relative import
Description
This PR reverts behavior in gatsby-transformer-remark so that date-like fields are serialized to string. This fixes a bug that some were experiencing where content types with a date string were sorted differently than content types with a date object, e.g. this change will revert the behavior so that the below two posts are sorted the same
Note that I do think this was a good change, so we'll want to consider perhaps reverting this and baking in better support in the resolvers for sorting these date fields in the future. But for now--we should revert to avoid breaking people's apps.
Related Issues
Fixes regression in #10924