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

Include the this object in method syntax #17981

Closed
Josh-Cena opened this issue Jul 5, 2022 · 18 comments
Closed

Include the this object in method syntax #17981

Josh-Cena opened this issue Jul 5, 2022 · 18 comments
Labels
Content:JS JavaScript docs

Comments

@Josh-Cena
Copy link
Member

Josh-Cena commented Jul 5, 2022

MDN URL

Taking https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map as example

What specific section or headline is this issue about?

Syntax

What information was incorrect, unhelpful, or incomplete?

The "syntax" is incomplete: map((element) => { /* ... */ }) does not highlight the fact that the method needs to have a correct this bound.

For example, if you do:

const { map } = [1, 2, 3];
map((e) => console.log(e));

It throws: TypeError: Array.prototype.map called on null or undefined

What did you expect to see?

For all methods that require the correct value of this, we should treat this as part of the parameters (it actually is, in many interpretations of ECMAScript semantics), and include it in both the "Syntax" and "Parameters" section.

For example, here we can write:

array.map((element) => {/* ... */})

And then in "Parameters", add:

  • array
    • : An Array, or array-like object. It will be the this context of the map method.

This is especially important for array and string methods, because they are intentionally designed to be generic, i.e. they don't require this to be an array/string exotic object, but can be anything that "behaves" like an array/string.

Do you have any supporting links, references, or citations?

https://tc39.es/ecma262/#sec-array.prototype.map:

The map function is intentionally generic; it does not require that its this value be an Array. Therefore it can be transferred to other kinds of objects for use as a method.

Do you have anything more you want to share?

No response

@github-actions github-actions bot added Content:JS JavaScript docs needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. labels Jul 5, 2022
@teoli2003
Copy link
Contributor

I would say, just this (without a mention in parameters)

array.map((element) => {/* ... */})

I think pretty everybody understands that the this will be bound to the array.

@teoli2003
Copy link
Contributor

And this as a "bad-example" (```js bad-example if I remember correctly) in the Examples section?

const { map } = [1, 2, 3];
map((e) => console.log(e));

It throws: TypeError: Array.prototype.map called on null or undefined

@Josh-Cena
Copy link
Member Author

Josh-Cena commented Jul 5, 2022

The more important part is that Array.prototype.map, as a function, does not require this to be an array.

Array.prototype.map.call({ 0: 1, 1: 2, 2: 3, length: 3 }, (e) => console.log(e));

({ map: Array.prototype.map, 0: 1, 1: 2, 2: 3, length: 3 }).map((e) => console.log(e));

So to some extent, the array is part of the parameters, because it is also meaningfully customizable.

As for example-bad, I'm not sure if it scales, because it applies to almost everywhere where the this is used.

@teoli2003
Copy link
Contributor

I see: what is the condition on a this for .map() to work on it?

@Josh-Cena
Copy link
Member Author

Josh-Cena commented Jul 5, 2022

Many methods in ECMA262 have an editorial note saying it's "intentionally generic". Such methods usually use duck-typing and only require the object to behave to the extent needed by the method. For example, map reads the length property of this, uses constructor.@@species to construct the new array, and then index-accesses each number between 0 and the length, so it only requires those properties to be accessible.

(For some supplementary material, last month I did an experiment which is published here: https://github.com/Josh-Cena/js-stalker)

@teoli2003
Copy link
Contributor

teoli2003 commented Jul 5, 2022

Thinking aloud.

We could keep the array in the syntax box, as this is the most common use case, and add a note below it. Something like:

Note: This method is intentionally generic and also works with objects that are not Array objects as long as they have a length property, have constructor.@@species accessible and allow for accessing using an index each number between 0 and the length.

This would also work for other intentionally generic features. We could link intentionally generic to a new glossary entry about duck typing with some more generic explanation.

@Josh-Cena
Copy link
Member Author

That sounds good as well.

@Josh-Cena Josh-Cena removed the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Jul 5, 2022
@hamishwillee
Copy link
Collaborator

Isn't the this context provided by thisArg?

@ghost
Copy link

ghost commented Jul 5, 2022

Isn't the this context provided by thisArg?

The thisArg is given to the mapper function, not Array#map itself.

@hamishwillee
Copy link
Collaborator

Thanks. I don't understand why the this belonging to the array matters, so I'll exit the discussion.

@ghost
Copy link

ghost commented Jul 5, 2022

For all methods that require the correct value of this, we should treat this as part of the parameters ..., and include it in both the "Syntax" and "Parameters" section.

Would this also apply to Web APIs?
For example, Request.prototype.clone():

## Syntax

request.clone()

## Parameters

- `this`
  - : The `Request` instance to be cloned.

## Exceptions

`TypeError`: `this` is not a `Request` object.
...

or if not, how should this be restricted?

And might introduction of explicit this parameters warrant mention of subclassing*, if applicable?
Thus introducing changes such as

arrayLike.map(fn, thisArg)

## Result

If `this` is an array subclass, an instance of the subclass, otherwise an Array.
...

* Plenty of methods access the constructor (and possibly Symbol.species) to subclass

@Josh-Cena
Copy link
Member Author

Yes, that's true. I'll probably start with JS APIs, but it equally applies to web APIs as well.

@ghost
Copy link

ghost commented Jul 6, 2022

Well, it's just that it would really become bloat, because I'm pretty sure that all Web API methods that have a this parameter simply throw if they aren't of the correct type.

@Josh-Cena
Copy link
Member Author

We can probably ignore that in "Parameters" in this case, but the fact that it throws makes it more essential to be demonstrated in the "Syntax" part.

@teoli2003
Copy link
Contributor

teoli2003 commented Jul 6, 2022

I'm not convinced to add this as a parameter at all, not in Web APIs for sure, not even in JS.

In 99% of the cases, this is obvious and only makes the documentation longer and therefore more complex and harder to understand.

As this is a major change to the structure of the pages, If you want to do this, you should start a discussion in mdn-community/discussion and reach a consensus there. Keep in mind that it is the holiday season and several maintainers are out until mid-August. I wouldn't claim a consensus before they come back.

For the moment, the only thing I agree we should do is to add a note for cases (in JS only) where methods are intentionally generic and can apply to other objects than the one they are defined on.

Once the discussion has started, we should close this issue, so we have a discussion at one place.

@Josh-Cena
Copy link
Member Author

This issue is mostly about the "Syntax" section, with "Parameters" as a favorable though not necessary enhancement. I'm okay to add it as a note/example everywhere, although I do believe that's even more space taken.

If you want to do this, you should start a discussion in mdn-community/discussion

Thought about it but it looks more like a bug because our current syntax is literally incorrect 😄 I would start a discussion about how we should formally document the semantics of this.

@Josh-Cena
Copy link
Member Author

I've started a discussion at mdn/mdn-community#148. Until we reach consensus there, we'll keep this closed.

@Josh-Cena Josh-Cena closed this as not planned Won't fix, can't repro, duplicate, stale Jul 6, 2022
@Josh-Cena Josh-Cena added the on hold Waiting on something else before this can be moved forward. label Jul 6, 2022
@teoli2003
Copy link
Contributor

Thank you!

@Josh-Cena Josh-Cena added revisit and removed on hold Waiting on something else before this can be moved forward. labels Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:JS JavaScript docs
Projects
None yet
Development

No branches or pull requests

3 participants