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

isPrivate/isStatic probably shouldn't be prefixed with "is" #417

Closed
domenic opened this issue Jul 26, 2021 · 15 comments · Fixed by #467
Closed

isPrivate/isStatic probably shouldn't be prefixed with "is" #417

domenic opened this issue Jul 26, 2021 · 15 comments · Fixed by #467

Comments

@domenic
Copy link
Member

domenic commented Jul 26, 2021

So far there's weak precedent in JavaScript specs and broader-but-with-exceptions precedent in web specs that "is" prefixes are reserved for functions asking questions about their arguments, and we don't use them otherwise. https://w3ctag.github.io/design-principles/#naming-booleans

Examples:

  • Functions asking questions about their arguments: Number.isNaN(arg), Array.isArray(arg).

  • Properties asking questions about their this value: regExp.multiline, temporalDate.inLeapYear, event.cancelable. (event.isTrusted is a notable exception on the web.)

  • An options argument being used as input into a spec-defined function: not sure of any ES precedents, but on the web we use new Event(type, { cancelable: true }), which makes sense to match the previous bullet (i.e. it'd be strange if you passed in isX as an argument and then had to consult the x property).

This spec is actually about an options argument being used as input to a user-defined function, which I'm not sure if we have any precedent for even on the web. But I think it makes sense to align with the general trend toward omitting "is" prefixes except in the functions-asking-question-about-their-arguments case.

@nicolo-ribaudo
Copy link
Member

I don't know why the authors chose isStatic and isPrivate, but difference with all these examples is that private and static are keywords. While since es5 keywords can be freely used as property names, you are forced to rename them when destructuring:

function decorator(value, { name, private: isPrivate, static: isStatic }) {
  // ...
}

@domenic
Copy link
Member Author

domenic commented Jan 4, 2022

I'd like to bump this issue to make sure it gets some attention before implementation starts in stage 3. I can understand if the committee decides to make an exception for property-names-that-would-otherwise-be-keywords, but if so that kind of precedent should be established willingly, not accidentally.

So, for example, it's worth thinking about what happens if a third boolean property is added to these descriptors, that is not a keyword: do you use is or not?

@leobalter
Copy link
Member

leobalter commented Jan 4, 2022

A good analogy that might help supporting this request is connecting these booleans with the property descriptors, also connected to a property.

@leobalter
Copy link
Member

In any case, my position here is neutral. I understand the goal for consistency and avoiding a new precedent and for that I'd be very supportive to avoiding the is* prefix. At the same time, the example from @nicolo-ribaudo shows the convenience the prefix provides for both private and static. Having an exception for these "future reserved words" can avoid the annoyance of some destructuring that fails or not if in strict or non-strict modes.

@pzuraq
Copy link
Collaborator

pzuraq commented Jan 7, 2022

I don't have a strong opinion here myself, while I think the benefit of is* prefixing is nice in this case, I also think that consistency with the platform would be good. Will add this to the next meeting agenda to discuss.

@js-choi
Copy link

js-choi commented Jan 7, 2022

With my developer hat on, I would strongly expect boolean flags to have no is prefix, and I would have been surprised if properties named with is did not refer to predicate methods.

When using them from the options argument, I would prefer to have developers either not use destructuring (e.g., options.private) or to rename the variables in destructuring (e.g., { private: privateFlag }).

@Andrew-Cottrell
Copy link

Given these flags only apply to class elements, would it be helpful to rename them to privateElement and staticElement?

Clearly, these names are less ergonomic; but, I wonder if they might help some people avoid attempting to use them with decorators intended to be called on classes or other JavaScript syntax forms (excluding class elements).

@ljharb
Copy link
Member

ljharb commented Jan 7, 2022

That implies they contain the element, as opposed to a boolean describing them.

@Andrew-Cottrell
Copy link

That implies they contain the element, as opposed to a boolean describing them.

Good point.

@bakkot
Copy link

bakkot commented Mar 29, 2022

The proposal got stage 3 without this issue arising again, and therefore with the current "isPrivate" names.

I personally totally forgot about this issue, and I suspect I was not the only one. I apologize. I'm hoping to raise this explicitly before this meeting is over (proposing to change it to "private"), or, failing that, at the next one, which is almost certain to be before anyone ships a feature of this size unflagged.

@pzuraq
Copy link
Collaborator

pzuraq commented Mar 30, 2022

We discussed this further in plenary and decided to update to private and static, I will be updating the explainer and spec accordingly.

@ljharb

This comment was marked as resolved.

@ljharb

This comment was marked as resolved.

@trusktr
Copy link
Contributor

trusktr commented Sep 17, 2022

Was it really worth losing destructuring simplicity here?

I have never been confused by is vs no is prefixes. With my developer hat on, I simply API docs: I would never assume something is a function just because it is prefixed with is.

@trusktr
Copy link
Contributor

trusktr commented Oct 4, 2022

Just mentioning that I end up always writing {private: isPrivate, static: isStatic } now because of this. Not so clean, but not end of the world either.

I bet millions that no one has blindly assumed Node.isConnected or HTMLElement.isContentEditable to be functions and arbitrarily tried to call them, or that if they did they quickly modified code and ended up staring at themselves in a mirror questioning their path in life. I could sure use some more money right now!

It wasn't necessary to remove is.

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.

9 participants