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

Deduplicate TypedArray ctor param description, add exceptions #20025

Merged
merged 2 commits into from
Oct 22, 2022

Conversation

Josh-Cena
Copy link
Member

Summary

Motivation

Fix #18764

Any duplicated content is a maintenance hassle and may be confusing to readers as well. Condensing them in the same place improves visibility.

This decision is further motivated by the addition of the "exceptions" section. If we don't do the deduplication, people won't be reading the TypedArray constructor page, and they won't know about the exception info. I don't want to duplicate that section just for visibility.

Supporting details

Related issues

Metadata

  • Adds a new document
  • Rewrites (or significantly expands) a document
  • Fixes a typo, bug, or other error

@Josh-Cena Josh-Cena requested a review from a team as a code owner August 27, 2022 04:38
@Josh-Cena Josh-Cena requested review from Elchi3 and removed request for a team August 27, 2022 04:38
@github-actions github-actions bot added the Content:JS JavaScript docs label Aug 27, 2022
- {{jsxref("TypeError")}}
- : Thrown in one of the following cases:
- A `typedArray` is passed but it is a [bigint](/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt) type while the current constructor is not, or vice versa.
- A `typedArray` is passed but the buffer it's viewing is detached, or a detached `buffer` is directly passed.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we've mentioned what "detached" means anywhere, although we did use this term in a few places. It's a complex topic; related: https://github.com/mdn/content/issues/19228

Copy link

@ghost ghost Aug 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that the glossary page about transferring was renamed from detachment, or just general uses of "detach" have been renamed to use transferring. I found it odd because of how I'd have rather authored some pages, to not have anything to link to..

Edit: see

@Elchi3
Copy link
Member

Elchi3 commented Aug 29, 2022

hm. I'm not sure about this one. I usually prefer reader convenience over authoring convenience with the goal that readers can get to useful information as quickly as possible. Often (not always) that can mean to duplicate content into relevant places.

@Josh-Cena
Copy link
Member Author

Josh-Cena commented Aug 29, 2022

I've done similar things in the past, e.g. #17817. I'm fine with either, but (1) it hasn't been particularly problematic for me to click on a link to view the description, considering I have to do that anyway for most of the TypedArray methods (2) accidentally making pages out-of-sync could be significantly more confusing for readers. It may even be in our interest to get rid of the individual typed array subclass pages altogether, a la https://github.com/mdn/content/discussions/18002, but that could be a step too big.

Meanwhile, if you absolutely want me to revert the parameter deduplication, I can do that. Do you think we need to duplicate the exceptions section in this case? I'm very worried about scalability. Even the spec does not separate the constructors' description at all.

@Elchi3
Copy link
Member

Elchi3 commented Aug 29, 2022

accidentally making pages out-of-sync could be significantly more confusing for readers.

Yes, I just think that wrongness or out-of-syncness is really an authoring problem. So if it is more work on the authoring side but with benefits for the readers, authors should pay that price (not always, it's a judgment call).

It may even be in our interest to get rid of the individual typed array subclass pages altogether, a la https://github.com/orgs/mdn/discussions/18002, but that could be a step too big.

Again coming from a reader's perspective (and for findability/search engine indexibility), my gut feeling is that it's a step too big.

Meanwhile, if you absolutely want me to revert the parameter deduplication, I can do that.

I lean towards keeping the parameter description. It's a judgment call, though. Other reviewers have opinions? @teoli2003 @wbamberg?

@Josh-Cena
Copy link
Member Author

I wouldn't have done it if I'm not to add the exceptions section. Seeing the exceptions duplicated again on each page is even sadder than the parameter description, because I intend to improve the error descriptions in the future (especially around transferred buffers), and I really don't want to do that across 11 pages every single time. It's just too error-prone. I don't know if that really makes the reader happy either—if I find that some similar content is presented on both pages, I would need to compare them side-by-side to see if there are some important differences that I may miss, only ending up realizing it's the same stuff CtrlCV'd. But that's just me. (I know we do it for some web APIs as well, like saying "check the global attributes" or "check the base class for this class's methods". I think we are in the same situation here)

@Josh-Cena
Copy link
Member Author

@wbamberg curious to hear your take on this... Given how long the parameters + exceptions get, duplicating it seems so cumbersome.

@Josh-Cena
Copy link
Member Author

👋 @wbamberg Any opinions on this? I was trying to work on #20019 and that somehow depends on the structure set by this PR (as well as the important information about typed array inheritance).

@wbamberg
Copy link
Collaborator

Sorry I missed this @Josh-Cena , I will take a look.

@@ -34,32 +34,7 @@ new BigUint64Array(buffer, byteOffset, length)

> **Note:** `BigUint64Array()` can only be constructed with [`new`](/en-US/docs/Web/JavaScript/Reference/Operators/new). Attempting to call it without `new` throws a {{jsxref("TypeError")}}.

### Parameters
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth keeping the parameters heading everywhere? Makes it easier for navigation.

Copy link
Member Author

@Josh-Cena Josh-Cena Oct 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a debate about this, but then if we keep it, we have to add "exceptions" as well—usually we put "parameters", "return value", and "exceptions" all under the "syntax" section. In this PR I'm moving all of the "syntax" content to the TypedArray base page, along with the addition of the "exceptions" subheading.

@wbamberg
Copy link
Collaborator

wbamberg commented Oct 21, 2022

Sorry, like I say, to be slow responding to this. I don't really know which is best tbh, I can see both sides here and I think neither are ideal. Maybe we should bring {{page}} back, ha ha (or, semi-seriously, have a more organized way of sharing content).

I agree that copy/pasting all the exception docs is unappealing but if we copy/paste the parameter descriptions it would be hard not to copy/paste exceptions too.

if I find that some similar content is presented on both pages, I would need to compare them side-by-side to see if there are some important differences that I may miss, only ending up realizing it's the same stuff CtrlCV'd.

I think this is a really good point that is often missed in these conversations. Duplication/transclusion is not necessarily better for readers.

So I guess I am (weakly) on the de-duplicating side?

If we do de-duplicate here, I do thing we should keep ### Parameters and ### Exceptions in the "target" pages and say something quite concise like:

### Parameters

See [`TypedArray`](/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray#parameters).

### Exceptions

See [`TypedArray`](/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray#exceptions).

We train people hard to scan for these headings.

@Josh-Cena
Copy link
Member Author

I've pushed the change. Lmk how it looks.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thank you @Josh-Cena !

@wbamberg wbamberg merged commit eb49396 into mdn:main Oct 22, 2022
@Josh-Cena Josh-Cena deleted the typedarray-exeception branch October 22, 2022 00:19
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

Successfully merging this pull request may close these issues.

TypedArray - Undocumented constructor errors
4 participants