-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
docs(plugins): add more information about plugins #3649
Conversation
✅ Build karma 528 completed (commit 067bed6617 by @devoto13) |
✅ Build karma 527 completed (commit 067bed6617 by @devoto13) |
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.
Awesome, thanks!
Offered a couple of suggestions.
|
||
## Preprocessors | ||
On the very high level you can think of Karma as an object where each key (a *DI token*) is mapped to a certain Karma object (a *service*). For example, `config` DI token maps to `Config` instance, which holds current Karma configuration. Plugins can request (or *inject*) various Karma objects by specifying a corresponding DI token. Upon injection a plugin can interact with injected services to implement their functionality. | ||
|
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.
Seem like this would be a good place to outline how the keys are added the to the DI map (Karma)
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.
Can you clarify what exactly do you mean by that?
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.
Just a suggestion that, along with the other great content here, a mention of how the DI table is populated would help demystify the DI.
docs/dev/05-plugins.md
Outdated
|
||
## Preparing a plugin for publishing | ||
|
||
The above examples used a so-called inline plugin form. It is convenient for examples or a tiny ad-hoc plugin, but when publishing a plugin for others to use, it is usually better to export an object from an NPM package to hide extra complexity from the users of the plugins. This also gives a possibility to register multiple plugins by importing a single NPM package. |
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 understand that you are trying to give a simple introduction by using the compact inline form, but it means later you need to issue a correction. I think using the prefered form first with more explanation and no backtracking is better. In particular I don't like the mixing of consumer and plugin code.
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've updated examples to use a separate file for the plugin code. I still demonstrate the usage of the plugin from the local file, so readers can copy-paste code and run without dealing with multiple NPM packages. Also removed the section on publishing as it is no longer needed. Hope this is enough.
There are already many [existing plugins]. Of course, you can write [your own plugins] too! | ||
## Activating Plugins | ||
|
||
Adding a plugin to the `plugins` array only makes Karma aware of the plugin, but it does not activate it. Depending on the plugin type you'll need to add a plugin name into `frameworks`, `reporters`, `preprocessors`, `middleware` or `browsers` configuration key to activate it. For the detailed information refer to the corresponding plugin documentation or check out [Developing plugins][developing plugins] guide for more in-depth explanation of how plugins work. |
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.
This is a common source of errors, not doing all of the steps. Can we have a preview earlier, just before "installing" which calls out the three steps? Then even better, each subsection of the three steps can call out:
Installing plugins is the first of three steps in their use....
Loading plugins is the second of three steps in their use...
Activating plugins is the third and final step in their uses...
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 agree with the point, but the current prose is not written as a step-by-step guide and I hesitate to rewrite the whole article to fix this comment. How about doing this in a separate PR later on?
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.
Perhaps my suggestion was not clear. I'm just suggesting that we have 4 sentences that help users realize that one step is not enough.
Changes: - Promote `require('karma-plugin')` form over `'karma-plugin'` form. Former makes it more clear that plugin is imported from an NPM package and it is a regular JS object, there is no magic behind it. This is inspired by karma-runner#3498 where user is not aware that it is even possible. This also should make it easier with plug'n'play package managers (like Yarn 2). - Explain that `plugins` array does not activate plugins, but only registers them to clarify karma-runner#1247 (comment). - Explain the plugin structure, DI and how to build a new plugin. - Re-arrange "Developing plugins" page to make it easier to add more information about every plugin type. Adding actual information should be done in the separate PRs though. Fixes karma-runner#1247
✅ Build karma 532 completed (commit 0cd86e8252 by @devoto13) |
✅ Build karma 531 completed (commit 0cd86e8252 by @devoto13) |
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.
such a great improvement let's get it in
🎉 This PR is included in version 6.1.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Changes: - Promote `require('karma-plugin')` form over `'karma-plugin'` form. Former makes it more clear that plugin is imported from an NPM package and it is a regular JS object, there is no magic behind it. This is inspired by karma-runner#3498 where user is not aware that it is even possible. This also should make it easier with plug'n'play package managers (like Yarn 2). - Explain that `plugins` array does not activate plugins, but only registers them to clarify karma-runner#1247 (comment). - Explain the plugin structure, DI and how to build a new plugin. - Re-arrange "Developing plugins" page to make it easier to add more information about every plugin type. Adding actual information should be done in the separate PRs though. Fixes karma-runner#1247
Changes:
require('karma-plugin')
form over'karma-plugin'
form. Former makes it more clear that plugin is imported from an NPM package and it is a regular JS object, there is no magic behind it. This is inspired by Feature Request/Proposal: Flag for specifying plugin directories/plug n' play support #3498 where user is not aware that it is even possible. This also should make it easier with plug'n'play package managers (like Yarn 2).plugins
array does not activate plugins, but only registers them to clarify Correct inline plugin example? #1247 (comment).Fixes #1247