Skip to content

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
.replace(/\n/g, ' '); // Replace new lines by spaces
}

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