Once you add these in here, it's possible for the user to override them. Should they be allowed to override them? I feel like ContextualMenuItem has gotten too large and should be split. ContextualMenuItem is the information that gets passed into the contextmenu, then something like ContextualMenuItemProps are the props that will get passed to the component that renders the item.

@dzearing any ideas?

EDIT:
Oops, just saw the previous comment. Disregard what I said here.

@joschect From my understanding, even if I didn't add those values in, wouldn't we still have the same problem, because of the any property field that already exists?

If that's the case, I think the only difference is whether or not we explicitly or implicitly inform the user that we have this information, or at least in this specific instance.

I agree that the ContextualMenuItem is getting rather large, but I also think we can leverage the [propertyName: string]: any prop on IContextualMenuItem. The user could pass 'aria-posinset': <#> and 'aria-setsize': <#> to menu items already and we should piggy back off of that when passing to onRender. This would allow to do everything we need without adding any new props

If we went this route, the only thing that would need to be thought is if we want to allow creators to be able to override these values for non custom renders... Currently if a user supplies those props they will be overridden by the base render functions. If we want to allow this value to be overridden, we could do the spread (talked about in other comments) and pass the updated item to all render fuctions

If we do proceed with this we will need to allow users to provide their own values for this. So you should do an expand like const itemWithProps = {... {positionInSet: focusableElementIndex, setSize: totalItemCount, hasCheckMarks: hasCheckMarks, hasIcon: hasIcons}, ...item} that way their passed in item values will override the ones you provide.

I agree that we should just spread the props, assign the value and allow for it to be overridden. You wouldn't need to put the other props inside of their own object though, it could look something like:
item.onRender( {posInSet: focusableElementIndex, setSize: totalItemCount, ...item}, this.dismiss)

I may not be understanding correctly, but it seems like we're mutating the item, which is a terrible idea. We shouldn't pollute the consumer input like this. having some additional props in the onRender stuff seems reasonable though.

Perfect, I switched to use the spread operator.

Do we currently have a need have a need for the hasCheckmarks or hasIcons? If not, it should only be added once it is needed

No we don't. They have been removed.

Did you mean for each key to be defined differently? (e.g. on is is string and the other is a string array)

Whoa, I actually don't know how those braces actually made it in there. They should not be there. The intention is to pass it in as an ordinary native attribute. Sorry about that.