[Proposal] Add `supportedDependencyTypes` documentation by bradlc · Pull Request #1585 · postcss/postcss

bradlc · 2021-05-18T11:04:42Z

This pull request adds documentation for a way that PostCSS runners can specify which dependency types they support. Now that we have more than one dependency type, I think it is important that plugins are aware of which dependency types are available to them, and which aren't.

My proposal is for runners to define a supportedDependencyTypes option when running PostCSS:

processor.process({
  from: file.path,
  to: file.path,
  supportedDependencyTypes: ['dependency', 'dir-dependency']
})

Then, plugins can read this from result.opts:

if (result.opts.supportedDependencyTypes.includes('dir-dependency')) {
  // register directory dependency
} else {
  throw Error('Runner does not support the required dependency types.')
}

This provides a better experience for PostCSS users, because plugins can present clear errors when required dependency types are not available. It also allows plugins to register dependencies with confidence, knowing that they will be handled by the runner. Currently there is no way to know that a dir-dependency message has been handled. This can lead to plugins silently not working correctly and frustrated users.

Here's another concrete example: if we added a new glob-dependency type (as discussed over on the Parcel repo) plugins could make a decision about which dependencies to register, based on which types the runner supports:

let pattern = '/src/**/*.html'
let supportsGlobDependencies = result.opts.supportedDependencyTypes?.includes('glob-dependency')
let supportsDirectoryDependencies = result.opts.supportedDependencyTypes?.includes('dir-dependency')

if (supportsGlobDependencies) {
  messages.push({ type: 'glob-dependency', glob: pattern })
} else if (supportsDirectoryDependencies) {
  messages.push({ type: 'dir-dependency', dir: getBase(pattern) })
} else {
  throw Error('Runner does not support the required dependency types.')
}

Some runners may not be able to support glob dependencies at all, and this would allow plugins to maximise compatibility.

This is just one idea and I am curious to hear what you think of it. Feedback and suggestions welcome, thanks!

ai · 2021-05-18T15:00:24Z

I do not like the idea that a plugin should throw an error. There are a lot of use case, when we do not have watch mode and dir-dependencies support mean nothing.

I can accept it only if we will find a better UX for plugins. But I am not sure that we need it.

bradlc · 2021-05-18T15:39:42Z

I do not like the idea that a plugin should throw an error.

What do you recommend a plugin do if it can't run in the user's environment?

[...] when we do not have watch mode and dir-dependencies support mean nothing.

I do not think that is the case when a runner has persistent caching like webpack 5 does. The cache is maintained across separate builds so the dependencies are taken into account.

ai · 2021-05-18T15:54:16Z

What do you recommend a plugin do if it can't run in the user's environment?

What plugin requires dependency messages to work not in watch mode?

I do not think that is the case when a runner has persistent caching like webpack 5 does. The cache is maintained across separate builds so the dependencies are taken into account.

Don’t forget about postcss-cli and gulp-postcss runners. They do not have caches and watch mode is rarely used.

bradlc · 2021-05-18T16:19:39Z

What plugin requires dependency messages to work not in watch mode?

Any plugin, when using a runner that uses persistent caching, like webpack 5 (with opt-in). Here's an example with instructions: https://github.com/bradlc/postcss-webpack5

ai · 2021-05-18T16:56:12Z

Any plugin, when using a runner that uses persistent caching, like webpack 5 (with opt-in). Here's an example with instructions: https://github.com/bradlc/postcss-webpack5

I am asking about the opposite question. If we run PostCSS plugin with dir-dependency messages in gulp-postcss (which has no any dependency messages support), what will be the problem?

bradlc · 2021-05-18T16:59:33Z

Ah sorry. So the problem would be that if my plugin reads the content from files in the example directory, it needs to re-run when files in that directory change. If a runner doesn't support dir-dependency then this won't happen, and my plugin won't work as expected.

ai · 2021-05-18T17:05:30Z

But gulp-postcss is often build CSS from the scratch without any caches. If you throw an error on no dir-dependency, plugin will not work in postcss-cli and gulp-postcss.

ai · 2021-05-18T17:06:34Z

docs/guidelines/plugin.md

 })
 ```

+### 3.2. Detect available dependency types


I suggest to not do it mandatory and only add supportedDependencyTypes to types

bradlc · 2021-05-18T17:24:17Z

But gulp-postcss is often build CSS from the scratch without any caches. If you throw an error on no dir-dependency, plugin will not work in postcss-cli and gulp-postcss.

Ooh I see what you mean now I think! So gulp-postcss will say "I don't support any dependency types" because it doesn't actually do anything with the messages (because it doesn't need to). Got it. That is a good point. What if gulp-postcss declared that it supports them even though it doesn't actually do anything with them? Not a very elegant solution though.

devongovett · 2021-05-30T18:18:53Z

Would it make sense to not throw when no dependency types are supported but just not emit anything? In that case, there's no watcher anyway, so there's no reason it should break.

ai · 2021-05-30T18:29:15Z

Yeap, let's be remove the error from examples. I think we should not add any rules to plugin guidelines. Only to runner guidelines.

DeMoorJasper · 2021-05-30T19:05:55Z

Mentioned an alternative to the glob/dir dependency in the Parcel PR that might remove the need for the functionality proposed in this PR

@bradlc Maybe you could merge the glob and dir dependency message into a singular dir-dependency message? This would probably simplify the logic quite a bit and not require any type of registering which dependency types are supported:

Currently
{ "type": "dir-dependency", "dir": "/src/" }
with glob (glob is relative to dir) - the glob would be optional so plugins can choose to add it or leave it out
{ "type": "dir-dependency", "dir": "/src/", "glob": "**/*.html" }

ai · 2021-05-31T01:26:52Z

I like glob as an extension!

bradlc · 2021-06-01T12:23:31Z

@DeMoorJasper I like it! 🙌 Opened a new pull request for that as an alternative to this one 👍

Add supportedDependencyTypes documentation

89598ef

ai reviewed May 18, 2021

View reviewed changes

bradlc mentioned this pull request May 27, 2021

Add support for PostCSS dir-dependency messages parcel-bundler/parcel#6299

Merged

bradlc mentioned this pull request Jun 1, 2021

Add glob message property documentation #1590

Merged

ai closed this Nov 17, 2021

Uh oh!

Conversation

bradlc commented May 18, 2021

Uh oh!

ai commented May 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bradlc commented May 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ai commented May 18, 2021

Uh oh!

bradlc commented May 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ai commented May 18, 2021

Uh oh!

bradlc commented May 18, 2021

Uh oh!

ai commented May 18, 2021

Uh oh!

ai May 18, 2021

Choose a reason for hiding this comment

Uh oh!

bradlc commented May 18, 2021

Uh oh!

devongovett commented May 30, 2021

Uh oh!

ai commented May 30, 2021

Uh oh!

DeMoorJasper commented May 30, 2021

Uh oh!

ai commented May 31, 2021

Uh oh!

bradlc commented Jun 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ai commented May 18, 2021 •

edited

Loading

bradlc commented May 18, 2021 •

edited

Loading

bradlc commented May 18, 2021 •

edited

Loading