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

feat(docz-core): updates to plugin hooks #431

Merged
merged 1 commit into from
Nov 14, 2018

Conversation

transitive-bullshit
Copy link
Contributor

@transitive-bullshit transitive-bullshit commented Oct 30, 2018

Description

This change adds two new plugin hooks to docz-core, supporting plugin functionality that was previously not supported.

It adds plugin.modifyFiles which gives plugins a chance to edit the list of mdx source files before docz starts processing them.

It also adds plugin.setConfigPromise which is the async analogue of plugin.setConfig. This allows plugins to run more dynamic setup processes such as modifying the docz config based on the project's filesystem state or a sub-build process.

Note that the naming of setConfig vs setConfigPromise is following the excellent precedent set by Webpack's plugin hooks which is explained here.

Review

One thing we'll need to add to this PR before merging is minor changes to the plugin docs to add these new methods.

Pre-merge checklist

All linting, builds, and tests pass locally.

Screenshots

There is no public functionality affected aside from the plugin endpoints exposed.


@pedronauck the use case for these additional hooks is explained in more detail in this WIP docz-plugin-storybook and the associated PR hydrateio/docz-plugin-storybook#4 which uses both the existing and new docz plugin hooks to automatically populate docz's entries with existing Storybook stories. 😄

We're pretty excited about the docz-plugin-storybook module and have considered multiple ways of hooking into both docz and storybook, with this PR being the net result. We believe these additional plugin hooks make a lot of sense for more use cases than just our own plugin.

Looking forward to hearing your thoughts!

Thanks!

@pedronauck
Copy link
Member

First of all, thanks for the contribution, it's a pretty good idea 🙏
About the hooks, I don't like so much setConfigPromise, I think that setConfigAsync is better. So, what do you think about let just one function setConfig and make the decision inside it if will resolve as a promise or not.

@transitive-bullshit
Copy link
Contributor Author

I think that makes a lot of sense.

If I'm understanding you correctly, we remove the extra hook, stay with setConfig but allow implementations to return Config or Promise<Config>? If so, that sounds great and I can make that change pretty easily 👍

@pedronauck
Copy link
Member

Yeah, that's exactly what I thought @transitive-bullshit 👏

@transitive-bullshit
Copy link
Contributor Author

@pedronauck updated 😄

This change adds two new plugin hooks to docz-core supporting
functionality previously not supported.

It adds plugin.modifyFiles which gives plugins the chance to edit the
list of mdx source files before docz starts processing them.

It also adds plugin.setConfigPromise which is the async analogue of
plugin.setConfig. This allows plugins to run more dynamic setup
processes such as modifying the docz config based on the project's
filesystem state or a sub-build process.

Note that the naming of setConfig vs setConfigPromise is following the
precedent set by Webpack's plugin hooks which is explained here:

https://webpack.js.org/api/plugins/#plugin-types
@transitive-bullshit
Copy link
Contributor Author

Just updated with latest from master.

@pedronauck happy to make any other changes you'd like to get this merged 😄

@transitive-bullshit transitive-bullshit changed the title feat(docz-core): add two new plugin hooks feat(docz-core): update plugin hooks (async setConfig and modifyFiles) Nov 4, 2018
@transitive-bullshit transitive-bullshit changed the title feat(docz-core): update plugin hooks (async setConfig and modifyFiles) feat(docz-core): updates to plugin hooks Nov 4, 2018
@pedronauck
Copy link
Member

hi @transitive-bullshit, thanks to the updated... I'll merge it today 🙏

@mergebandit
Copy link

Hey Pedro - we're actively working around this at the moment - would love to get this merged. Let us know if there's anything else you'd like us to do!

@pedronauck pedronauck changed the base branch from master to dev November 14, 2018 01:04
@pedronauck pedronauck merged commit f4a122f into doczjs:dev Nov 14, 2018
@pedronauck
Copy link
Member

@mergebandit sorry to the too late response here guys, I'm merging and releasing some patch soon 🙏

@transitive-bullshit transitive-bullshit deleted the feature/add-plugin-hooks branch November 14, 2018 06:46
@transitive-bullshit
Copy link
Contributor Author

No worries @pedronauck -- thanks so much & looking forward to the release 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants