-
-
Notifications
You must be signed in to change notification settings - Fork 834
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
Remove lodash
from core
#2827
Remove lodash
from core
#2827
Conversation
-3.4 kB from bundle (-1.65 kB gzipped) |
Could you not include imports reordering in this PR? That, if at all, should be done outside it. I'm not in love with copying lodash methods simply to save a few kilobytes, as that means we then are responsible for maintaining those functions, but I guess the ones chosen shouldn't be an issue... |
We need a polyfill for iOS 11 and below. I think using a native method with this polyfill is better than having our own function instead, even if the bundle size is ~150B more.
69456f3
to
882f742
Compare
I've switched out the flatten util for the native browser This does make the bundle slightly bigger, but I think it's worth it for the likely better-optimised native browser code and the simplicity of just calling |
Just going to see what we gain/lose by using |
So it's 366 073 bytes with |
const realDepth = isNaN(depth as any) ? 1 : depth; | ||
|
||
return realDepth | ||
? Array.prototype.reduce.call( | ||
this, | ||
function (acc, cur) { | ||
if (Array.isArray(cur)) { | ||
acc.push.apply(acc, flat.call(cur, realDepth - 1)); | ||
(acc as Array<any>).push.apply(acc, flat.call(cur, realDepth - 1)); |
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.
Can this not be done in the function (acc: )
argument type hinting?
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 would love to if I could figure out what Typescript was continuously complaining about...
I think Array.prototype.reduce.call
expects the input to be anything, as it's a generic function, so it just uses any
as the types for the accumulator and current value, which plays havoc when we want to use .push
.
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.
Yeah I can't easily read that xD
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.
Y e a h...
I might just type the function args then do as any
on the callback to prevent any complaints. That makes it a bit cleaner.
Progresses flarum/issue-archive#179
Changes proposed in this pull request:
Removed
lodash-es
from our dependencies, and have replaced them with a custom util, a polyfill for a native browser method, and thethrottle-debounce
library.Reviewers should focus on:
These replacements don't function identically to the original library's, but should suit our use cases.
Confirmed