-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
refactor: remove unneeded object #1286
Conversation
Will now unneeded -> does it make a difference? |
This saves a few bytes (~16) in bundle size and some heap memory (approx. 64 bytes per element using I'd still consider merging the PR due to the widespread use of this library, although I wouldn't create an extra release with just this small change. |
} | ||
const [t, setT] = useState(getT()); // seems we can't have functions as value -> wrap it in obj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what was the reason for this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the original author of the code just tried useState(getT())
with getT()
returning the i18n.t function. React detected that the parameter passed to useState was a function and thought it was a lazy-init function (useState(() => initialValue)
). So it was calling i18n.t with no args and used the return value as initial value, which was probably null or undefined.
Basically to store a function in a state you have to have a function which returns the function, e.g. useState(() => (args) => console.log(args))
and setState(() => (args) => console.log(args))
lgtm |
thank you...was published in react-i18next@11.8.12 |
Minor optimizations
Works by switching to
useState(() => initialValue)
andsetState((prevState) => nextState)
Checklist
npm run test