Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

feat: add plugins detection #111

Merged
merged 6 commits into from
Jan 25, 2021
Merged

feat: add plugins detection #111

merged 6 commits into from
Jan 25, 2021

Conversation

erezrokah
Copy link
Contributor

@erezrokah erezrokah commented Jan 22, 2021 •
edited

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

  • Update the jsdoc comments
  • Update docs

@github-actions github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Jan 22, 2021
ehmicky
ehmicky reviewed Jan 22, 2021
View reviewed changes
src/frameworks/next.json Outdated Show resolved Hide resolved
@erezrokah erezrokah marked this pull request as ready for review January 22, 2021 16:19
ehmicky
ehmicky reviewed Jan 22, 2021
View reviewed changes
src/plugins.js Outdated

const getPlugins = function (plugins, { nodeVersion }) {
const ajv = new Ajv({})
ajv.addKeyword(MIN_NODE_VERSION_KEYWORD)
Copy link
Contributor

@ehmicky ehmicky Jan 22, 2021 •
edited

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.

Copy link
Contributor Author

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
ehmicky approved these changes Jan 22, 2021
View reviewed changes
Copy link
Contributor

@ehmicky ehmicky left a 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?

@erezrokah
Copy link
Contributor Author

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 Next.js version I would keep that logic inside the plugin.

@erezrokah erezrokah merged commit db4e8f5 into master Jan 25, 2021
@erezrokah erezrokah deleted the feat/plugins_support branch January 25, 2021 10:38
ehmicky
ehmicky reviewed Jan 25, 2021
View reviewed changes
src/frameworks/grunt.json
"env": {}
"env": {},
"plugins": [],
"plugins": []
Copy link
Contributor

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 ⬆️

@erezrokah erezrokah mentioned this pull request Jan 25, 2021
Merged
@erezrokah erezrokah mentioned this pull request Feb 2, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ehmicky ehmicky ehmicky approved these changes

Assignees
No one assigned
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add detection logic for the Next.js plugin
2 participants
@erezrokah @ehmicky