Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
`Drupal.throwError` is rethrow the error message. If you are passing String into `Drupal.throwError`, there's no more useful error stack for debugging.
It need not to check param type explicitly but should discourage its usage in our API design.
Proposed resolution
- Remove String type in JSDoc
Remaining tasks
- Review patch
User interface changes
- N/A
API changes
- Docs change. Do not affect any code usages.
Data model changes
- N/A
Comment | File | Size | Author |
---|---|---|---|
Drupal.throwError.patch | 447 bytes | droplet | |
|
Comments
Comment #2
droplet CreditAttribution: droplet commentedComment #16
smustgrave CreditAttribution: smustgrave at Mobomo commentedDon't expect any failures but running 10.1 tests.
Comment #17
smustgrave CreditAttribution: smustgrave at Mobomo commentedPasses 10.1 and I can't see a reason to not add it but will leave it to the committers
Comment #18
xjmThanks for finding and resurrecting this issue!
To review whether a documented typehint is correct, we need to look at the method code and the usages: Is it possible to pass a string, and if so, does any code actually do that?
I grepped and found the following non-documentation references in core:
The code of the method itself is simple:
As far as I can tell, that code does not restrict the type of
error
. Since I'm not the world's most pro JavaScript developer, I also looked up the docs forthrow
and confirmed that it can be not only an object but also a string, a number, a boolean... So string is currently allowed, and changing the code to disallow it would be a pretty dramatic BC break.I checked the six usages in core. Five of them were indeed simply throwing
Error()
, and the sixth (from the recent change to get JS errors to fail tests) is a bit more complicated:All that said, it's entirely possible, even probable, that contrib and custom code is already using this method with a string message. So while throwing
Error
might be better, we can't just removestring
from the docs.We have three options:
Error
is preferable for developer experience and better debugging.Error
in Drupal 11.This is too JavaScript-y for me to make the decision, so I defer to the expertise of those that have it.