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.

Sign up for GitHub

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

Jump to bottom

feat(extend/injector): bring up new extend Injector #4049

Merged
merged 13 commits into from
Jun 16, 2020
Merged

feat(extend/injector): bring up new extend Injector #4049

merged 13 commits into from
Jun 16, 2020

Conversation

SukkaW
Copy link
Member

@SukkaW SukkaW commented Jan 2, 2020 •
edited by stevenjoezhang

What does it do?

Implementation of #4047. Should only be merged after #4048 is fixed.

How to test

git clone -b feat-extend-inject https://github.com/sukkaw/hexo.git
cd hexo
npm install
npm test

Screenshots

Pull request tasks

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

Issue resolved: #3086
Issue resolved: #4047

@SukkaW SukkaW requested a review from a team January 2, 2020 17:37
@SukkaW SukkaW changed the title feat(extend/injectpr): bring up new extend Injector feat(extend/injector): bring up new extend Injector Jan 2, 2020
@coveralls
Copy link

coveralls commented Jan 2, 2020 •
edited

Coverage Status

Coverage increased (+0.02%) to 97.728% when pulling 93f12c6 on SukkaW:feat-extend-inject into da5723c on hexojs:master.

SukkaW
SukkaW commented Jan 5, 2020
View reviewed changes
lib/extend/injector.js Outdated Show resolved Hide resolved
@SukkaW SukkaW force-pushed the feat-extend-inject branch 3 times, most recently from f44cdfd to 0213f5f Compare January 5, 2020 16:01
@SukkaW
Copy link
Member Author

SukkaW commented Jan 7, 2020

@hexojs/core It is ready to be reviewed now.

dailyrandomphoto
dailyrandomphoto reviewed Jan 7, 2020
View reviewed changes
lib/extend/injector.js Outdated Show resolved Hide resolved
lib/extend/injector.js Outdated Show resolved Hide resolved
lib/plugins/filter/after_render/injector.js Outdated Show resolved Hide resolved
lib/plugins/filter/after_render/injector.js Outdated Show resolved Hide resolved
@dailyrandomphoto
Copy link
Member

LGTM!

@SukkaW SukkaW requested a review from a team January 8, 2020 03:09
@SukkaW SukkaW added the enhancement New feature or request label Jan 8, 2020
@SukkaW
Copy link
Member Author

SukkaW commented Jan 31, 2020 •
edited

@dailyrandomphoto @jiangtj

I have refactored the Injector recently.

A new Injector.exec() method has been added (libs/plugins dir should only be used for internal plugins that are based on hexo.extend).
Also, Injector is now separate from Filter. Injector will insert code before after_router_render filter's execution.

The test on my local machine with a dummy hexo site looks good as well:

image

CI build will be fixed in #4102. The PR is ready for review.

@SukkaW
refactor(injector): use new data structure
6695d78 
Apply suggestions from code review by @dailyrandomphoto
@SukkaW
feat(injector): support page.layout
38ebdc3
@SukkaW
refactor(injector): simplify the filter
789614d 
Apply code suggestions from code review by @dailyrandomphoto
@SukkaW
refactor(injector): split injector from filter
18421ce 
Make sure injector is executed before after_route_render
@SukkaW
Copy link
Member Author

SukkaW commented Feb 6, 2020

Rebased and ready for review.

@SukkaW
refactor(injector): add exec() method
8cdaf0b 
- Remove injectorFilter from plugin dir
- Move cache into Injector Class
- Remove decache
@SukkaW
Copy link
Member Author

SukkaW commented May 31, 2020 •
edited

Resolve conflict completed & ready for review.

@SukkaW SukkaW added this to the 5.0.0 milestone Jun 16, 2020
@SukkaW SukkaW merged commit d117819 into hexojs:master Jun 16, 2020
This was referenced Jun 16, 2020
Closed
Merged
@SukkaW SukkaW mentioned this pull request Jun 23, 2020
Closed
@SukkaW SukkaW mentioned this pull request Jul 25, 2020
Merged
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jiangtj jiangtj jiangtj approved these changes

@dailyrandomphoto dailyrandomphoto Awaiting requested review from dailyrandomphoto

@curbengh curbengh Awaiting requested review from curbengh

@yoshinorin yoshinorin Awaiting requested review from yoshinorin

@stevenjoezhang stevenjoezhang Awaiting requested review from stevenjoezhang

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

The proposal of new type of extension: Inject Any official way to insert HTML codes once per file?
4 participants
@SukkaW @coveralls @dailyrandomphoto @jiangtj