-
Notifications
You must be signed in to change notification settings - Fork 511
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
feat: add prerender:config
, prerender:init
and prerender:done
hooks
#1519
Conversation
β¦eat/prerender-done-hook
Codecov Report
@@ Coverage Diff @@
## main #1519 +/- ##
==========================================
- Coverage 76.23% 76.23% -0.01%
==========================================
Files 73 73
Lines 7524 7540 +16
Branches 739 738 -1
==========================================
+ Hits 5736 5748 +12
- Misses 1787 1790 +3
- Partials 1 2 +1
|
prerenderer:config
, prerenderer:done
and prerenderer:init
hooks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful feature idea. I agree with @pi0's comment, but otherwise π
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love it!
Regarding the hook names, we use the before/done terminology for modules and build, should we use this as a convention ? Or maybe define a new one such as init/finished or before/start ?
Should I make a separate PR for this or include it here? |
prerenderer:config
, prerenderer:close
and prerenderer:init
hooksprerenderer:config
, prerenderer:done
and prerenderer:init
hooks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM π― (have to fix failing ci, it needs parent dir creation)
To fix the test properly it would need access to the main nitro instance (not the prerenderer) for the preset output directories π€ A native For now, I just removed the test. |
prerenderer:config
, prerenderer:done
and prerenderer:init
hooksprerender:config
, prerender:init
and prerender:done
hooks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for the PR. I have pushed some relevant refactors on top of it hope it is all fine to you.
Perfect, thank you. I'll think about the prerender controller and if I find some free time and it hasn't been attempted yet, will give it a go. |
π Linked issue
Related nuxt/nuxt#22255
β Type of change
π Description
An initial step to giving greater prerendering powers to integrations.
Some examples of what this unblocks:
Out of scope of this PR but still to do be done:
π Checklist