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

perf(meta_generator): drop cheerio #3671

Merged
merged 7 commits into from
Aug 21, 2019
Merged

Conversation

curbengh
Copy link
Contributor

@curbengh curbengh commented Aug 16, 2019

What does it do?

Less breaking alternative to #3669
x-post from that PR:

From the benchmark performed by @SukkaW , we can see cheerio—used by meta_generator filter—incurs significant perf cost. The feature was initially introduced by #3129.

Despite effort to improve the situation (#3667), the improvement was only minuscule. Hence, we try to avoid cheerio if possible.

How to test

git clone -b metagen-cheerio https://github.com/curbengh/hexo.git
cd hexo
npm install
npm test

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.

@coveralls
Copy link

coveralls commented Aug 16, 2019

Coverage Status

Coverage decreased (-0.002%) to 97.152% when pulling fb42a5b on curbengh:metagen-cheerio into 49c7d4a on hexojs:master.

@stevenjoezhang
Copy link
Member

Maybe it's not necessary to use the xhml syntax?

<meta name="generator" content="Hexo %s" />

<meta name="generator" content="Hexo %s">

@curbengh
Copy link
Contributor Author

btw, if anyone's worry about html in a code block, the brackets are encoded,

<figure class="highlight html"><table><tbody><tr><td class="code"><pre><span class="line"><span class="tag">&lt;<span class="name">head</span>&gt;</span><span class="tag">&lt;/<span class="name">head</span>&gt;</span></span><br></pre></td></tr></tbody></table></figure>

SukkaW added a commit to theme-suka/performance-test that referenced this pull request Aug 16, 2019
SukkaW
SukkaW previously approved these changes Aug 16, 2019
@curbengh
Copy link
Contributor Author

curbengh commented Aug 17, 2019

I forgot to add a unit test where <head> already have meta[generator]. Maybe that's why coveralls decreased.
I'll add it.

Edit: added

decreased coverage may be due to shorter lines/functions.

SukkaW
SukkaW previously approved these changes Aug 17, 2019
Copy link
Member

@SukkaW SukkaW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@curbengh
Copy link
Contributor Author

squashed some commits.

@SukkaW SukkaW added this to the v4.0.0 milestone Aug 18, 2019
@SukkaW SukkaW self-requested a review August 18, 2019 07:25
@curbengh curbengh mentioned this pull request Aug 18, 2019
4 tasks
@curbengh curbengh changed the title refactor(meta_generator): drop cheerio perf(meta_generator): drop cheerio Aug 18, 2019
@tomap tomap merged commit 651f34b into hexojs:master Aug 21, 2019
@curbengh curbengh deleted the metagen-cheerio branch August 21, 2019 07:54
thom4parisot pushed a commit to thom4parisot/hexo that referenced this pull request Jan 17, 2020
* refactor(meta_generator): drop cheerio

* fix(meta_generator): append to </title>

* test(meta_generator): remove irrelevant head tag test

* test(render): disable meta_generator

* fix(meta_generator): do not append if there is existing tag,

regardless the value of 'content'

* test(open_generator): test existing generator tag

* test(meta_generator): tag should be added only once

Previous commit only works on post, not page
This reverts commit 325f303.
curbengh pushed a commit to curbengh/curbengh.github.io that referenced this pull request Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants