Values from string and markdown widgets come out as either empty string or undefined, depending on whether the field has been edited

[papandreou]

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

bug

- What is the current behavior?

String and markdown widgets that are empty produce different data values, depending on whether the field has been edited.

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

1. Define a non-required string field in config.yml:

      -
        label: non-required string field
        name: nonrequiredstring
        widget: string
        required: false

2. Create a new item in the collection
3. Don't touch the non-required string field at all
4. Click Publish

Observe that the nonrequiredstring field isn't in the frontmatter of the resulting markdown file.

5. Create a new item in the collection
6. Enter some text in the non-required string field, then delete the text again so the field is blank
7. Click Publish

Observe that frontmatter of the resulting markdown file contains nonrequiredstringfield: ''

- What is the expected behavior?

That the resulting value is the same in both cases, preferably undefined.

The issue can also be observed for string and markdown widgets showing up in editor components. Also if you open an existing item, remove the text in a field, then Publish.

- Please mention your versions where applicable.

Netlify CMS version: 1.9.2
Browser version: Chrome 67.0.3396.87/OSX

Node.JS version: 10.4.1
Operating System: Mac OSX 10.13.5 (High Sierra)

[tech4him1]

Likely related to #1432.

[papandreou]

@tech4him1, that one only affects markdown fields, though? This one can also be reproduced with string widgets.

[tech4him1]

Yes, you are correct. This whole issue (including boolean fields) is related to #977, but that was supposedly fixed. Maybe we need to initialize the Redux store with the defaults as well, not just the widgets.

[tech4him1]

#1126 changed the value for when the widget was empty, but not the initial value in Redux. I'm still not sure we really want the value to show up if the user has never edited it, but that needs to be discussed. It's definitely not consistent.

@erquhart @talves @Benaiah Any thoughts?

[erquhart]

So perhaps:

1. The default value for every widget is undefined.
2. A widget can provide it's own default value, probably via registry.
3. The default value for a widget instance can be overridden via config.
4. undefined values, along with their keys, are filtered out of the output when persisting.

[tech4him1]

2. A widget can provide it's own default value, probably via registry.

This may be cleaner as part of the widget itself, as suggested in #725.

3. The default value for a widget instance can be overridden via config.

We'll need to decide what the defaults should be for built-in widgets as well. Point number 2 is the only thing really new here, IMO.

For example, with a string widget, what should happen:

• by default, with no changes?
• when the string widget is changed to empty?

This may need to be different for optional and required fields -- the main point is that the two questions above are consistent.

I hope that makes sense, I'm not trying to argue here. :)

[erquhart]

We need to allow dynamic default values to be set for hidden widgets, where the React component is never loaded, so the separation is going to happen, and doing the same thing two ways will get confusing. Passing the default value to the CMS also gives the core more control over the experience without limiting extension developers. I expect we'll start thinking of a widget as bigger than just a React component as we move into more robust behaviors.

I'm even starting to view our tendency to do everything via component methods as an anti-pattern, because it limits our use of that functionality. E.g., only the control component itself can validate widget output because we use a validate method on the component, when ideally the CMS should be able to validate a value wherever it needs to, without digging through the React tree, grabbing refs, etc. as we do currently.

What's new here is having a single set of conditions governing defaults and establishing the following consistent expectations:

1. undefined value means both key and value are omitted in the output (falsey doesn't matter) - I believe this along with my # 4 point above addresses your points on built-in widget defaults
2. doubling down on not doing everything through the component
3. providing a single, API based approach for widget developer to set the default value for their widget

[erquhart]

Hmm your point on what happens after a value is changed and then deleted to an empty string isn't really addressed. Almost feels like we need our own set of "falsey" values for those that you'd just never expect to persist. Like false is a specific value that should be persisted, but I don't think there's a great reason to persist an empty string. Maybe we should filter out undefined, null, and empty strings?

In that case other value types like numbers and objects would be left to the widget developer to ensure a "blank" value (0 or {} for example) gets changed back to something that gets filtered out, like null, on change.

Thoughts?

[pungggi]

but I don't think there's a great reason to persist an empty string

Yes at least one.. in gatsby you define a graphql query from say the source of a yaml file from netlify cms.. but then all of sudden the user deletes a field and the build is broken because gatsby cannot find that field anymore..

[erquhart]

I'd really love for that to be fixed on the Gatsby side, Netlify CMS isn't the only entry point where that pain is felt. But I'm not 100% against it, it makes sense to provide some way to output an empty string or any other falsey value if the implementor wants.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[TidyIQ]

Yes at least one.. in gatsby you define a graphql query from say the source of a yaml file from netlify cms.. but then all of sudden the user deletes a field and the build is broken because gatsby cannot find that field anymore..

This can be fixed in Gatsby by explicity defining the schema, as per the documentation. Instead of returning an empty string if a field is edited then deleted (which causes issues for any widgets other than strings), it should just remove the field entirely from the yaml/markdown/json file so that a GraphQL query with explicitly defined schema will return the value as null instead of throwing a type mismatch error.

[erquhart]

@TidyIQ thanks for adding that, much better approach than outputting empty strings.

[erquhart]

Closing in favor of #995.