-
-
Notifications
You must be signed in to change notification settings - Fork 68
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
add plugin id help for config's #103
add plugin id help for config's #103
Conversation
@@ -113,14 +115,24 @@ const plugin = opts => { | |||
const usedPlugins = supportedFeatures.map(feature => feature.plugin); | |||
usedPlugins.push(stagedAutoprefixer); | |||
|
|||
const internalPlugin = () => { |
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 was broken.
It's not possible to have both OnceExit
and plugins
in the same PostCSS plugin.
These need to be chained.
I do not know the exportsTo
plugin so I can't easily add a test for it to double check that everything is fine now.
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 for this! This is great and I think it'll help in the future to ease the plugin pack usage. Just left a minor comment. I'm sure I'm missing something.
$font: system-ui; | ||
|
||
.test-variable { | ||
font-family: $font; | ||
font-family: system-ui, -apple-system, Segoe UI, Roboto, Ubuntu, Cantarell, Noto Sans, sans-serif; | ||
} |
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 change confuses me a bit. Why is this needed?
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.
It confused me too.
tape contents for this test :
'insert:after:array': {
message: 'supports { stage: 1, after: { "lab-function": [ require("postcss-simple-vars") ] } } usage',
options: {
stage: 1,
insertAfter: {
'lab-function': [
require('postcss-simple-vars')()
]
},
features: {
'lab-function': {
unresolved: 'ignore'
}
}
},
expect: 'insert.after.expect.css'
},
I replaced the color-mod
stuff with lab-function
to resolve the warnings.
I think that insertAfter
looks up the index of the plugin (lab-function
) and tries to add the other plugin after it.
If the feature is not found I think it was never inserted.
So when color-mod
was removed postcss-simple-vars
was no longer applied correctly in this test.
Now that I have changed it to lab-function
it actually works again.
Although probably not as designed because of plugins not having an order in PostCSS 8
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.
Hmm we should probably add another issue to test this meaningfully since I don't think this test proves anything right now
@@ -11,6 +11,10 @@ export function pluginIdHelp(featureNamesInOptions, root, result) { | |||
const byId = mostSimilar(featureName, featureNames); | |||
const byPackage = mostSimilar(featureName, packageNames); | |||
|
|||
// TODO : | |||
// 1. create markdown docs with the plugin id's |
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.
outside of scope of this change.
But I want to already indicate were in the code this can be mentioned to users.
partial fix for : csstools/postcss-preset-env#156
Also see : #86 (reply in thread)
Uses a levenshtein distance to possibly suggest the correct plugin.
Tests failed because we still had some that were using
color-mod
and this then throw warnings with the new help stuff.Maybe the entire
insertBefore
insertAfter
stuff is outdated since this is no longer of PostCSS plugins run? But this is outside the scope of this change.