This content originally appeared on Jake Archibald's blog and was authored by Jake Archibald's blog
Here's an old pattern that seems to be making a comeback:
// Convert some numbers into human-readable strings: import { toReadableNumber } from 'some-library'; const readableNumbers = someNumbers.map(toReadableNumber);
Where the implementation of toReadableNumber is like this:
export function toReadableNumber(num) { // Return num as string in a human readable form. // Eg 10000000 might become '10,000,000' }
Everything works great until some-library is updated, then everything breaks. But it isn't some-library's fault – they never designed toReadableNumber to be a callback to array.map.
Here's the problem:
// We think of: const readableNumbers = someNumbers.map(toReadableNumber); // …as being like: const readableNumbers = someNumbers.map((n) => toReadableNumber(n)); // …but it's more like: const readableNumbers = someNumbers.map((item, index, arr) => toReadableNumber(item, index, arr), );
We're also passing the index of the item in the array, and the array itself to toReadableNumber. This worked fine at first, because toReadableNumber only had one parameter, but in the new version:
export function toReadableNumber(num, base = 10) { // Return num as string in a human readable form. // In base 10 by default, but this can be changed. }
The developers of toReadableNumber felt they were making a backwards-compatible change. They added a new parameter, and gave it a default value. However, they didn't expect that some code would have already been calling the function with three arguments.
toReadableNumber wasn't designed to be a callback to array.map, so the safe thing to do is create your own function that is designed to work with array.map:
const readableNumbers = someNumbers.map((n) => toReadableNumber(n));
And that's it! The developers of toReadableNumber can now add parameters without breaking our code.
The same issue, but with web platform functions
Here's an example I saw recently:
// A promise for the next frame: const nextFrame = () => new Promise(requestAnimationFrame);
But this is equivalent to:
const nextFrame = () => new Promise((resolve, reject) => requestAnimationFrame(resolve, reject));
This works today because requestAnimationFrame only does something with the first argument, but that might not be true forever. A extra parameter might be added, and the above code could break in whatever browser ships the updated requestAnimationFrame.
The best example of this pattern going wrong is probably:
const parsedInts = ['-10', '0', '10', '20', '30'].map(parseInt);
If anyone asks you the result of that in a tech interview, I recommend rolling your eyes and walking out. But anyway, the answer is [-10, NaN, 2, 6, 12], because parseInt has a second parameter.
Option objects can have the same gotcha
Chrome 90 will allow you to use an AbortSignal to remove an event listener, meaning a single AbortSignal can be used to remove event listeners, cancel fetches, and anything else that supports signals:
const controller = new AbortController(); const { signal } = controller; el.addEventListener('mousemove', callback, { signal }); el.addEventListener('pointermove', callback, { signal }); el.addEventListener('touchmove', callback, { signal }); // Later, remove all three listeners: controller.abort();
However, I saw an example where instead of:
const controller = new AbortController(); const { signal } = controller; el.addEventListener(name, callback, { signal });
…they did this:
const controller = new AbortController(); el.addEventListener(name, callback, controller);
As with the callback examples, this works today, but it might break in future.
An AbortController was not designed to be an option object to addEventListener. It works right now because the only thing AbortController and the addEventListener options have in common is the signal property.
If, say in future, AbortController gets a controller.capture(otherController) method, the behaviour of your listeners will change, because addEventListener will see a truthy value in the capture property, and capture is a valid option for addEventListener.
As with the callback examples, it's best to create an object that's designed to be addEventListener options:
const controller = new AbortController(); const options = { signal: controller.signal }; el.addEventListener(name, callback, options); // Although I find this pattern easier when multiple things // get the same signal: const { signal } = controller; el.addEventListener(name, callback, { signal });
And that's it! Watch out for functions being used as callbacks, and objects being used as options, unless they were designed for those purposes. Unfortunately I'm not aware of a linting rule that catches it (edit: looks like there's a rule that catches some cases, thanks James Ross!).
TypeScript doesn't solve this
Edit: When I first posted this, it had a little note at the end showing that TypeScript doesn't prevent this issue, but I still got folks on Twitter telling me to "just use TypeScript", so let's look at it in more detail.
TypeScript doesn't like this:
function oneArg(arg1: string) { console.log(arg1); } oneArg('hello', 'world'); // ^^^^^^^ // Expected 1 arguments, but got 2.
But it's fine with this:
function twoArgCallback(cb: (arg1: string, arg2: string) => void) { cb('hello', 'world'); } twoArgCallback(oneArg);
…even though the result is the same.
Therefore TypeScript is fine with this:
function toReadableNumber(num): string { // Return num as string in a human readable form. // Eg 10000000 might become '10,000,000' return ''; } const readableNumbers = [1, 2, 3].map(toReadableNumber);
If toReadableNumber changed to add a second string param, TypeScript would complain, but that isn't what happened in the example. An additional number param was added, and this meets the type constraints.
Things get worse with the requestAnimationFrame example, because this goes wrong after a new version of a browser is deployed, not when a new version of your project is deployed. Additionally, TypeScript DOM types tend to lag behind what browsers ship by months.
In my opinion, TypeScript should enforce the number of arguments passed to a callback, just as it does with regular functions. Or at least, there should be an option for this.
Things are a bit tougher when it comes to option objects:
interface Options { reverse?: boolean; } function whatever({ reverse = false }: Options = {}) { console.log(reverse); }
You could say that TypeScript should warn if the object passed to whatever has properties other than reverse. But in this example:
whatever({ reverse: true });
…we're passing an object with properties like toString, constructor, valueOf, hasOwnProperty etc etc since the object above is an instance of Object. It seems too restrictive to require that the properties are 'own' properties (that isn't how it works at runtime), but maybe TypeScript could add some allowance for properties that come with Object.
I'm a fan of TypeScript, this blog is built using TypeScript, but it does not solve this problem.
Thanks to my podcast husband Surma for proof-reading and suggestions.
This content originally appeared on Jake Archibald's blog and was authored by Jake Archibald's blog
Jake Archibald's blog | Sciencx (2021-01-29T01:00:00+00:00) Don’t use functions as callbacks unless they’re designed for it. Retrieved from https://www.scien.cx/2021/01/29/dont-use-functions-as-callbacks-unless-theyre-designed-for-it/
Please log in to upload a file.
There are no updates yet.
Click the Upload button above to add an update.