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

Empty children arrays #1

Closed
eush77 opened this issue Dec 20, 2015 · 1 comment
Closed

Empty children arrays #1

eush77 opened this issue Dec 20, 2015 · 1 comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 👶 semver/patch This is a backwards-compatible fix 🐛 type/bug This is a problem

Comments

@eush77
Copy link
Contributor

eush77 commented Dec 20, 2015

This is a question and a suggestion regarding this part of Unist readme (emphasis mine):

Unist nodes:

  • may have either a value property set to a string or a children property set to an array of one or more Unist nodes;

I read it as “Unist nodes may have a children property, in which case it is guaranteed that its length is ≥1”. If this is correct, then it follows that both retext, mdast, and hast violate this specification by producing trees with empty children arrays:

> retext.parse('')
{ type: 'RootNode', children: [] }
> mdast.parse('')
{ type: 'root',
  children: [],
  position: { start: { line: 1, column: 1 }, end: { line: 1, column: 1 } } }
> mdast.parse('#')
{ type: 'root',
  children: [ { type: 'heading', depth: 1, children: [], position: [Object] } ],
  position: { start: { line: 1, column: 1 }, end: { line: 1, column: 2 } } }
> hast.parse('')
{ type: 'root', children: [] }

If I haven't missed anything then I guess this requirement should be relaxed to include empty children (or removed if it doesn't require anything) or, alternatively, retext, mdast, and hast should be fixed to never output nodes with empty children arrays. The latter seems more problematic (the obvious workaround is returning null on empty input but I feel that it's better for parsers to always output a valid syntax tree) so I opened the issue here.

@wooorm
Copy link
Member

wooorm commented Dec 20, 2015

Oeh, thanks @eush77 for bringing up this point. I think this is wrong, and it should be as follows:

 *   must have a `type` property set to a to its namespace semantic
     `string`;

 *   may have either a `value` property set to a `string` or a `children`
-    property set to an array of one or more `Unist` nodes;
+    property set to an array of zero or more `Unist` nodes;

 *   may have a `data` property set to a `JSON.stringify`able object;

...as I agree with your points 😄

@wooorm wooorm closed this as completed in a6d3a03 Dec 23, 2015
@wooorm wooorm added 🐛 type/bug This is a problem 👶 semver/patch This is a backwards-compatible fix labels Feb 6, 2016
@wooorm wooorm added ⛵️ status/released 🗄 area/interface This affects the public interface labels Aug 12, 2019
@wooorm wooorm added the 💪 phase/solved Post is done label Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 👶 semver/patch This is a backwards-compatible fix 🐛 type/bug This is a problem
Development

Successfully merging a pull request may close this issue.

2 participants