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

Mark kids optional in list #75

Open
bendtherules opened this issue Dec 21, 2019 · 2 comments · May be fixed by #88
Open

Mark kids optional in list #75

bendtherules opened this issue Dec 21, 2019 · 2 comments · May be fixed by #88

Comments

@bendtherules
Copy link

bendtherules commented Dec 21, 2019

Ref line -

export const list = (
ordered: "ordered" | "unordered",
kids: Children
): Parent => ({
...nodeWithChildren("list", kids),
ordered: ordered === "ordered"
});

IMO, kids parameter should be marked as optional. This is just a typing change, no change in impl. needed.
I understand that just empty lists have no equivalent in markdown (and hence probably the existing decision), but it also enables other usecases (like building step-by-step). Also, it is somewhat in the same spirit as allowing empty kids in heading.

Use case -

This will allow users to build lists in two steps, like:

l = list("ordered"); 
l.children.push(listitem("abc"));

This is how things are typically handled in some converters (to md).
(Its not a major issue though, can be skipped using empty array as child)

@mike-north
Copy link
Owner

Given that the "vanilla JS" implementation of "starting with an empty list and building later" is

const list = [];
list.push("item");

having to explicitly pass an empty array as an argument feels analogous to the empty array declaration.

I see value in catching errors (through type-checking) where users fail to pass children as an argument, and am not yet convinced that the benefits associated with what you're proposing

-list("ordered", []);
+list("ordered");

would be enough to create a net improvement.

@bendtherules
Copy link
Author

Yes, i understand your point and it's more of a consistency issue than anything else.

But then shouldn't kids be required for other nodes like paragraphs also?

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

Successfully merging a pull request may close this issue.

2 participants