Skip to content
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

Replace lodash with native API #3753

Closed
16 tasks done
SukkaW opened this issue Oct 8, 2019 · 9 comments
Closed
16 tasks done

Replace lodash with native API #3753

SukkaW opened this issue Oct 8, 2019 · 9 comments

Comments

@SukkaW
Copy link
Member

SukkaW commented Oct 8, 2019

Feature Request

The idea came from hexojs/site#1139 (comment) & #3677.

There is an article suggests we don't need lodash anymore.

According to benchmark below we can see es6 performance more or less as same as Lodash, in term of CPU, Memory or Handling time, at least in browser.

There are also some benchmark of lodash vs es6 and es6 is definitely much faster in browser:

IMHO, we might consider replace at least some lodash with native js.

A guide about how to replace lodash with es6: https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore

A list of files that requires lodash:

hexo

We might start with lib located under lib/plugins/

warehouse

cc @curbengh @segayuu

@SukkaW SukkaW added feature-request enhancement New feature or request labels Oct 8, 2019
@SukkaW
Copy link
Member Author

SukkaW commented Oct 8, 2019

Also, benchmarks are needed to see if the generation becomes faster after we remove lodash.

@SukkaW SukkaW added this to the Candidate next release milestone Oct 8, 2019
@NoahDragon NoahDragon mentioned this issue Oct 8, 2019
53 tasks
@curbengh
Copy link
Contributor

curbengh commented Oct 9, 2019

I created a project to help us learn from each other's PRs in this task.

Currently, Hexo exposes lodash function globally. If we manage to refactor everything, we might consider dropping lodash entirely. That is definitely a breaking change, perhaps part of v5 roadmap.

@SukkaW
Copy link
Member Author

SukkaW commented Oct 9, 2019

Currently, Hexo exposes lodash function globally. If we manage to refactor everything, we might consider dropping lodash entirely. That is definitely a breaking change, perhaps part of v5 roadmap.

@curbengh We might drop in helpers & filters first, which might not cause any breaking changes and can be done in v4.

@SukkaW
Copy link
Member Author

SukkaW commented Oct 27, 2019

Currently, there is no better ways to replace lodash.merge or lodash.asignIn with vanilla JS. I assume it is the reason we should use lodash.

@curbengh
Copy link
Contributor

curbengh commented Oct 28, 2019

One annoyance I found with Object.assign() is that it doesn't replace key with undefined value, especially when assigning a default config for a plugin. a key will have undefined/null value when the value is left empty, even the default config has empty keys. As such, I always need to check for undefined and assign default value again.

lodash's _.defaults (used previously in hexo) doesn't need that extra step.

// Sample _config
const hexo = { aaa: { bbb: undefined } }

hexo.aaa = Object.assign({ bbb: 'ccc' }, hexo.aaa)
// undefined

hexo.aaa = _.defaults({ bbb: 'ccc' }, hexo.aaa)
// 'ccc'

hexo.aaa = _.defaultsDeep({ bbb: 'ccc' }, hexo.aaa)
// 'ccc'

hexo.aaa = _.merge({ bbb: 'ccc' }, hexo.aaa)
// 'ccc'

hexo.aaa = _.assign({ bbb: 'ccc' }, hexo.aaa)
// undefined

hexo.aaa = _.assignIn({ bbb: 'ccc' }, hexo.aaa)
// undefined

hexo.aaa = _.extend({ bbb: 'ccc' }, hexo.aaa)
// undefined

A quirk with _.defaults() is that the parameters need to be reversed, otherwise it replaces the user config...

// User config
const hexo = { aaa: { bbb: 'custom' } }

hexo.aaa = _.defaults({ bbb: 'ccc' }, hexo.aaa)
// 'ccc'

hexo.aaa = _.defaults(hexo.aaa, { bbb: 'ccc' })
// 'custom'

But it doesn't replace null though 😞

const hexo = { aaa: { bbb: null } }

hexo.aaa = _.defaults(hexo.aaa, { bbb: 'ccc' })
// null

@SukkaW
Copy link
Member Author

SukkaW commented Oct 28, 2019

@curbengh Yeah, only replace _.defaults with Object.assign when there is no undefined will be met.
Also, when there is no prototype thing need to be merged, Object.assign could be used to replace _.assignIn as well, just as I did in #3785

@SukkaW
Copy link
Member Author

SukkaW commented Dec 1, 2019

I have tried Object.getPrototypeOf, Reflect.getPrototypeOf even deprecated __proto__, but none of them could match the performance of lodash.merge.

@SukkaW
Copy link
Member Author

SukkaW commented Dec 1, 2019

I have nearly finished:

function extend(...args) {
  for (let i < args.length - 1; i >= 0; --i) {
    for (const key in args[i])
      args[0][key] = args[i][key];
  }

  return args[0];
}

The extend() below could merge multiple one-level objects along with its prototype. But it just can not perform a deep object merge.

And unfortunately, the locals in lib/theme/view has multiple level.


The config, theme config along with lodash itself was bind to Locals as prototype:

hexo/lib/hexo/index.js

Lines 359 to 365 in e423773

Locals.prototype.config = config;
Locals.prototype.theme = Object.assign({}, config, theme.config, config.theme_config);
Locals.prototype._ = _;
Locals.prototype.layout = 'layout';
Locals.prototype.env = this.env;
Locals.prototype.view_dir = join(this.theme_dir, 'layout') + sep;
Locals.prototype.site = this.locals.toObject();

@SukkaW
Copy link
Member Author

SukkaW commented Dec 16, 2019

Hexo has completely replace lodash with native JavaScript after #3969 is merged, so the issue can be closed now.

I have added a separate item Remove _ (lodash) from Locals to Roadmap (#2492). Since removing lodash from Locals should considered as a Breaking Changes, we will finish it in Hexo 5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants