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

Add advice about read-only lists #292

Merged
merged 3 commits into from
May 13, 2021
Merged

Add advice about read-only lists #292

merged 3 commits into from
May 13, 2021

Conversation

atanassov
Copy link
Contributor

@atanassov atanassov commented Mar 3, 2021

The added recommendation is about the types of static lists to be considered for API design.
Addresses #50


Preview | Diff

The added recommendation is about the types of static lists to be considered for API design.
Addresses #50
@atanassov atanassov requested review from hober and LeaVerou March 3, 2021 17:09
@LeaVerou
Copy link
Member

LeaVerou commented Mar 3, 2021

What happened to ReadOnlyArray? Was it folded into ObservableArray as a special case?

@cynthia
Copy link
Member

cynthia commented May 11, 2021

@plinss and I discussed this in a breakout during the F2F. We think the ObservableArray recommendation is a bit strange - the observable properties are definitely not desirable by every case where an attribute is needed, and there are cases where read-only is desirable (e.g. in an attribute) - but this PR doesn't seem to cover that.

Also, @plinss mentioned he wasn't so sure about the return being restricted to only sequences. There are cases where other array types are more desirable, but this feels a bit restrictive.

@annevk
Copy link
Member

annevk commented May 11, 2021

@LeaVerou I'm not aware of a ReadOnlyArray proposal, but https://heycam.github.io/webidl/#idl-observable-array does indeed satisfy that use case (it's basically up to API designers to decide what kind of operations they permit).

@domenic was it considered to have default implementations of delete and set an indexed value so that the readonly case would be consistent across APIs? That does strike me as something that is missing, but I might also be missing something. 😊

@domenic
Copy link
Member

domenic commented May 11, 2021

The plan was to create a ReadonlyArray Web IDL type which has those defaults and uses the same underlying infrastructure as ObservableArray.

@annevk
Copy link
Member

annevk commented May 11, 2021

Ah okay, so I guess we're mainly waiting for a consumer of that. Thanks!

@atanassov
Copy link
Contributor Author

Had another round of CR with @plinss and @cynthia and arrived with the final wordsmithing addressing all concerns for this PR. Thank you all for the time.

@atanassov atanassov merged commit 1c64d38 into main May 13, 2021
@atanassov atanassov deleted the atanassov-ro-lists branch May 13, 2021 00:38
hober pushed a commit that referenced this pull request Apr 20, 2023
* Add advice about read-only lists

The added recommendation is about the types of static lists to be considered for API design.
Addresses #50

* Addressing CR feedback

* One last nit addressed
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.

6 participants