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(open_graph): only require cheerio when content contains '<img' #3670

Closed
wants to merge 1 commit into from

Conversation

SukkaW
Copy link
Member

@SukkaW SukkaW commented Aug 15, 2019

What does it do?

only require cheerio when content contains '<img'

How to test

git clone -b BRANCH https://github.com/USER/hexo.git
cd hexo
npm install
npm test

Screenshots

#3663 (comment)

Pull request tasks

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

only require cheerio when content contains '<img'
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0008%) to 97.155% when pulling d6dce10 on SukkaW:lazy-cheerio into 3176fe0 on hexojs:master.

@SukkaW SukkaW changed the title feat(open_graph): require cheerio when content contains '<img' feat(open_graph): only require cheerio when content contains '<img' Aug 16, 2019
@@ -59,12 +58,16 @@ function openGraphHelper(options = {}) {
if (!images.length && content) {
Copy link
Contributor

@curbengh curbengh Aug 16, 2019

Choose a reason for hiding this comment

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

Is if (!images.length && content && content.includes('<img')) safe? I'm wondering if content can be null. Let me try with an empty file.


No issue/error with an empty file. Even with an empty file, it still contains basic stuff defined in a theme. So, I assume content will never be empty.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe some syntax error might cause content to be null, I'm not sure.

@SukkaW SukkaW added this to the v4.0.0 milestone Aug 18, 2019
@curbengh curbengh mentioned this pull request Aug 18, 2019
4 tasks
@curbengh curbengh changed the title feat(open_graph): only require cheerio when content contains '<img' perf(open_graph): only require cheerio when content contains '<img' Aug 18, 2019
@curbengh curbengh self-requested a review August 21, 2019 08:09
@curbengh
Copy link
Contributor

My preference is on #3680

@SukkaW
Copy link
Member Author

SukkaW commented Aug 22, 2019

Superseded by #3680

@SukkaW SukkaW closed this Aug 22, 2019
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.

3 participants