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

Datetime widget is wrapping date in single quotes #1271

Closed
charlotte-l opened this issue Apr 16, 2018 · 8 comments · Fixed by #1296
Closed

Datetime widget is wrapping date in single quotes #1271

charlotte-l opened this issue Apr 16, 2018 · 8 comments · Fixed by #1296

Comments

@charlotte-l
Copy link

- Do you want to request a feature or report a bug?
Bug

- What is the current behavior?
Dates generated by the Datetime widget are wrapped in single quotes in the document's front matter.
Example:
date: '2018-03-21T11:14:49+00:00'

- If the current behavior is a bug, please provide the steps to reproduce.

  1. Create a new entry in the CMS
  2. Click "Save"
  3. Change status to "Ready"
  4. Click "Publish"
  5. Note that in the markdown file created, the front matter date is wrapped in single quotes

No other front matter fields which we are using are subject to this issue.

- What is the expected behavior?
Previously the date was not wrapped in single quotes. As the date is now interpreted as a string, this breaks interactions with metalsmith (which requires dates to be in a date format, not as a string).
Example:
date: 2018-03-21T11:14:49+00:00

- Please mention your CMS, node.js, and operating system version.
Netlify CMS v1.5.0

@tech4him1
Copy link
Contributor

tech4him1 commented Apr 16, 2018

@charlotte-l Can you check what CMS version this happened in? Has it just been since v1.5.0? If you aren't sure what version it changed in, please try v1.3.1 and v1.4.0 as well.

@erquhart
Copy link
Contributor

@tech4him1 I believe we intentionally dropped the date type from the yaml parser right? Looks like some SSG's need it after all. Hadn't thought much about type requirements in output until this started being reported.

@tech4him1
Copy link
Contributor

tech4him1 commented Apr 17, 2018

@erquhart No, that PR has not been merged yet. This was possibly affected by #1143.

@erquhart
Copy link
Contributor

erquhart commented Apr 17, 2018

Oh okay, gotcha. So that's tough - the date widget was supposed to be returning a string the whole time, the documented default value, but was instead returning the same default value as datetime, and folks reporting this were relying on the bug behavior. A blank string as format should be the recommended fix, but I'm betting the CMS will consider that falsy and ignore it.

@tech4him1
Copy link
Contributor

tech4him1 commented Apr 17, 2018

moment().format('') still returns a string for me though, have you tested it? It may be reasonable to allow users to just set format: false, in which case we return a date object, but that still doesn't address the backwards compatibility issue.

@ghost
Copy link

ghost commented Apr 17, 2018

I use the date field to sort data inside my Jekyll collection. Having dates within '' breaks Jekyll build process. Please address this issue soon, unable to use the CMS because of this

@erquhart
Copy link
Contributor

erquhart commented Apr 17, 2018

@tech4him1 yep it's definitely that PR - folks have been relying on two bugs this whole time. This line used to evaluate true if:

  1. you did not set a format, and
  2. you did not change the value in the date field

When both of these conditions are met, a Date object was returned, but since that PR, a format is always provided by the widget, so a date object is never returned.

Defining the breaking change

Currently, the documented behavior is that the date widget returns a formatted string, but the experience of most users is that it returns a date object. I'm inclined to consider returning to the documented behavior as a breaking change, since that's the net impact for most users.

Dates should not be strings

Here's another thing: Most users should never need a formatted date string. I don't know why I didn't realize this sooner - their templates will apply formatting via their static site generator. Formatting the string from the CMS is probably an edge case.

Proposal

Because the prior behavior was so iffy, I'd rather us improve from where we are to address this rather than simply revert the commit at fault. The desired behavior should be:

  • A date object is always returned if no format is provided in the config
  • The same type is always returned both initially and on change
  • This applies to both date and datetime widgets
  • Docs should be updated to reflect this behavior
  • Changelog should record this as a breaking change for clarity

Thoughts?

@tech4him1
Copy link
Contributor

We'll also need to note which file formats will serialize to a string anyway. YAML and TOML have an explicit Date format, but JSON doesn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants