My custom hook useTyper.js
ended up as a bit of a mess and hard to read.
It's mimicking a nested for-loop but using React.useRef
s to hold on to the indexes between renders. The indexes are the current positions of the outer and inner loop, those often named i
and j
in traditional nested for loops.
Here's a fancy code sandbox for the mess below.
One thing I'm particularly unhappy with is the naming of stringIdx
and stringsIdx
. Any suggestion for a change?
useTyper.js
import React from "react";
import useInterval from "./useInterval.js";
/**
* A simple typewriter that prints strings, one character after another
* @param {Array<string>} strings - Array of strings to type
* @param {function<string>} set - A useState setter to update the current string value
* @param {number} defaultDelay - Delay between updates
*/
const useTyper = (strings, set, defaultDelay = 100) => {
const [delay, setDelay] = React.useState(defaultDelay);
// index counters, normally named i and j if this were an imperative loop
const stringIdx = React.useRef(0);
const stringsIdx = React.useRef(0);
const textTyper = () => {
const currentString = strings[stringsIdx.current];
set(currentString.substr(0, stringIdx.current + 1));
if (stringIdx.current > currentString.length) {
// Done with string
stringsIdx.current += 1;
stringIdx.current = 0;
if (stringsIdx.current > strings.length - 1) {
setDelay(null); // Ended.
}
}
stringIdx.current += 1;
};
useInterval(textTyper, delay);
};
export default useTyper;
For completion, this is the useInterval
hook being used by useTyper
.
It can be referenced from one of Dan Abramovs blog posts.
useInterval.js
import React from 'react';
const useInterval = (callback, delay) => {
const savedCallback = React.useRef();
// Remember the latest callback.
React.useEffect(() => {
savedCallback.current = callback;
}, [callback]);
// Set up the interval.
React.useEffect(() => {
function tick() {
savedCallback.current();
}
if (delay !== null) {
let id = setInterval(tick, delay);
return () => {
clearInterval(id);
};
}
}, [delay]);
};
export default useInterval;
Here is how the useTyper
hook can be used.
import React, { useState } from "react";
import "./styles.css";
import useTyper from "./hooks/useTyper.js";
export default function App() {
const [placeHolder, setPlaceHolder] = useState("");
const placeHolders = [
"First input suggestion",
"Second input suggeston",
"Creativity flows... ",
"Alright, lets leave it here."
];
useTyper(placeHolders, setPlaceHolder);
return (
<div className="App">
<div className="container">
<input
id="formElement"
autoComplete="off"
autoFocus
size="20"
type="text"
placeholder={placeHolder}
/>
</div>
</div>
);
}
Again, here's the link to the code sandbox.
1 Answer 1
Looks to be fairly clean code.
Suggestions
- Use string::slice or string::substring versus string::substr.
substr
isn't deprecated but also isn't recommended for use.
Warning: Although
String.prototype.substr(...)
is not strictly deprecated (as in "removed from the Web standards"), it is considered a legacy function and should be avoided when possible. It is not part of the core JavaScript language and may be removed in the future. If at all possible, use thesubstring()
method instead.
setCurrentString(currentString.slice(0, currentStringIndex.current + 1));
- Provide clearer parameter names, i.e.
stringArray
versus juststrings
, andsetString
orsetCurrentString
versusset
.set
alone can be confusing as it isn't very descriptive, i.e. "is it a mathematical set?" By naming a function with verbNoun it is clearer to a reader what it is and what it does.
const useTyper = (stringsArray, setCurrentString, defaultDelay = 100) => {
stringIdx
andstringsIdx
can cause mental gymnastics looking for the 's' to differentiate the two. Isolated from one another the names aren't terrible, but when used together within the same function it is more demanding. The names should also align closer to the variable they indicate an index for. Based on the previous parameter naming suggestion I recommendstringArrayIndex
andcurrentStringIndex
.- You also appear to do an extra iteration on the
currentString
with the conditional testcurrentStringIndex.current > currentString.length
. You can increment to the next string when the current string's index is equal to the length,currentStringIndex.current >= currentString.length
. - Similar conditional test for checking for the end of the strings array, when the index is. greater then or equal to the length you can quit.
stringArrayIndex.current >= stringArray.length
const useTyper = (stringArray, setCurrentString, defaultDelay = 100) => {
const [delay, setDelay] = React.useState(defaultDelay);
// index counters, normally named i and j if this were an imperative loop
const currentStringIndex = React.useRef(0);
const stringArrayIndex = React.useRef(0);
const textTyper = () => {
const currentString = stringArray[stringArrayIndex.current];
setCurrentString(currentString.slice(0, currentStringIndex.current + 1));
if (currentStringIndex.current >= currentString.length) {
// Done with string
stringArrayIndex.current += 1;
currentStringIndex.current = 0;
if (stringArrayIndex.current >= stringArray.length) {
setDelay(null); // Ended.
}
}
currentStringIndex.current += 1;
};
useInterval(textTyper, delay);
};
Good job, nice hook.
-
1\$\begingroup\$ Excellent feedback! I ended up with this codesandbox with your exact suggestions, but: I removed the
+ 1
in the call tosetCurrentString
since it made the subsequent strings start with two characters printed instead of one. \$\endgroup\$HenrikSN– HenrikSN2020年06月29日 07:49:30 +00:00Commented Jun 29, 2020 at 7:49 -
1\$\begingroup\$ @HenrikSN Ah, good catch, I hadn't noticed that and likely thought it was my machine. I hadn't messed with the timer delay at all. \$\endgroup\$Drew Reese– Drew Reese2020年06月29日 16:14:02 +00:00Commented Jun 29, 2020 at 16:14