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

Normative: Return this on %TypedArray%.prototype.set #565

Closed
wants to merge 1 commit into from

Conversation

leobalter
Copy link
Member

Even if this change is super small, it's good to have more feedback, mostly from implementors.

Historically, %TypedArray%#set returns undefined in the Khronos spec, I believe that's the reason we have it this way.

I've talked to some developers with background experience on TypedArrays and WebGL and asked if this change would be useful. They confirmed it's a handy change. Plus, it allows chainable calls and makes it a bit more consistent with the other JS methods, like TypedArrays or even Map#set.

The main and only concern is performance. I informally asked a few implementors and it looks the impact wouldn't be a problem, if any. I would like to have more feedback on this part.

@domenic
Copy link
Member

domenic commented May 10, 2016

I am against this. I prefer preserving command query separation for APIs, even if we have notably failed to do so in a few other cases.

This requires a proposal that advances through the process, of course.

@claudepache
Copy link
Contributor

a bit more consistent with [...] even Map#set.

It’s ironic to mention possible consistency with Map#set, because the Map interface is internally very inconsistent, with its three mutation methods (set, delete and clear) returning three different types of result.

@leobalter
Copy link
Member Author

leobalter commented May 11, 2016

these mutation methods has different goals. My focus is on methods to add new items. I used Map#set as an example as I remember I wrote a specific test for the return value on test262, I missed this behavior on the tests for %TypedArray%#set.

TypedArrays are not Maps and are not used for the same level of work. In anyway, if you consider they are native APIs, I would expect something similar.

The following data shouldn't add much value to this discussion, as TypedArrays are from their kind, but was my first motivation:

Even Array#push does not return undefined, but that's not a good example on the matters of returned values.


The DataView#set* methods return undefined too, but I believe this previous discussion is necessary before going forward on them.

@leobalter
Copy link
Member Author

@domenic: I prefer preserving command query separation for APIs, even if we have notably failed to do so in a few other cases.

I understand the preference but I like JS being a multi-paradigm language. This change offers an interesting alternative without breaking anything.

This requires a proposal that advances through the process, of course.

I guessed it bad. I agree this need consensus, but thought it could be solved on this PR. If I need to make a proposal for this, this discussion may help to at least collect some data, even to tell me it's worth the time.

@bterlson
Copy link
Member

Agree this is probably a proposal thing. We haven't used the PR process for minor feature requests before, at least, but if people would like to entertain something like this as a PR we could discuss whether it's appropriate in Munich.

Re: the content of this proposal, seems ok to me so long as it doesn't break anything (and it seems very unlikely it would).

@allenwb
Copy link
Member

allenwb commented May 11, 2016

I'm pretty sure that this pattern (and perhaps this specific API) has been discussed at TC39 meetings before and that the consensus was that for consistency we should continue to follow the legacy pattern which as @domenic pointed out above is essentially the command-query separation pattern.

It would sure be nice if we had a more accessible record of such decisions. Anybody want to digest all the old meeting notes?

@leobalter
Copy link
Member Author

It would sure be nice if we had a more accessible record of such decisions.

It would be great.

Now I'm currently testing the Dataview set methods and it seems there's more to change if I want to move it forward. Before any change proposal - or none, perhaps - I'll read and search the old meeting notes.

@leobalter leobalter closed this May 12, 2016
@leobalter leobalter deleted the ta-set branch May 12, 2016 19:24
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 this pull request may close these issues.

None yet

5 participants