-
Notifications
You must be signed in to change notification settings - Fork 20
feat: add plugins detection #111
Conversation
src/plugins.js
Outdated
|
|
||
| const getPlugins = function (plugins, { nodeVersion }) { | ||
| const ajv = new Ajv({}) | ||
| ajv.addKeyword(MIN_NODE_VERSION_KEYWORD) |
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.
Should those two lines be done in the top-level scope?
ajv compiles JSON schemas under the hood, which is a little slow. It only does it the first time though (it is cached), so re-using the ajv instance between calls has a good performance advantage.
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.
Good point - I was probably too strict with not doing anything during require here
ehmicky
left a comment
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.
Great solution. Looks great!
I am guessing you are thinking of adding the other conditions for the Next.js plugin (next.js minimum version, etc.) as additional custom JSON schema keywords?
This PR puts us in a good place to implement additional conditions so we could do that - for |
| "env": {} | ||
| "env": {}, | ||
| "plugins": [], | ||
| "plugins": [] |
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.
@erezrokah I just noticed the duplicate there ⬆️
Fixes #109.
We could also have our own syntax for specifying conditions where a plugin should be used instead of extending
ajv.See deploy preview link https://deploy-preview-111--framework-info.netlify.app/
Todo