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
Extends and standardize dependency message #1537
Comments
Parcel 2 has APIs for:
I couldn't figure out what |
Chiming in for Tailwind CSS — for some stuff we are working on it is super helpful if we have a way to register preferably globs but at least directories as dependencies, as we need to re-build when new files are created that live in certain directories. Perfect world for us is being able to do this and be notified any time any matches change, are deleted, or new files are added that match the glob:
I know postcss-cli already supports this, but I suspect not really deliberately, as the paths are just passed to chokidar which does support globs. As far as I know webpack does not support any type of glob dependency, but you can register files or directories, which means if you can parse globs in postcss-loader then you can simulate it closely enough. I believe Vite only supports files currently, not directories or globs, but I don't know enough about the internals to know if support directories would be meaningfully simpler than supporting globs. I think in an ideal world we would be able to push out globs from within PostCSS and build tools would support that, but it's also important to be practical so if that is extremely complex or impossible in any of the popular build tools it might be a good idea to settle for files and directories. |
I spoke with @yyx990803 about what would/wouldn't be doable in Vite and he confirmed that supporting globs is not a problem at all, he just doesn't want to invest in it unless it's standard and expected behavior, which is why I think it's good we are having this discussion to formalize the API from PostCSS's perspective 👍🏻 |
As @adamwathan pointed out, postcss-cli does support globs (admittedly by accident, but we can make that explicit). However, worth noting that postcss-cli currently only watches for file changes and doesn't even watch for new files that fall under the input glob (let alone dependencies). The issue is postcss/postcss-cli#161; it'll be fairly involved to add, and I just don't have the bandwidth to do it myself. Happy to review PRs if someone else steps up for it, though. |
Also not understanding what you have in mind with a |
Supporting globs is not hard, but it definitely degrades performance. @adamwathan Can you describe why you need watch globs? Because when you do glob you known which files/directories you are use...
It's all about performance. Another problem with glob is that it is not standardized, unfortunately, different systems work differently, even different packages work different, and nobody can say what is right. |
@alexander-akait Yeah I can explain! We are building a new version of Tailwind that generates CSS on demand based on the contents of your template files (React components, HTML files, Vue files, whatever you are using), similar to how the ACSS atomizer works. To do this, the end user needs to configure Tailwind with a list of globs pointing to all of their template files, which looks something like this: // tailwind.config.js
module.exports = {
files: [
'./components/**/*.{js,jsx,ts,tsx}',
'./pages/**/*.{js,jsx,ts,tsx}',
'./public/**/*.html',
]
// ...
} We can parse these globs and register the files directly ourselves but then we can't rebuild when new files are added that match the globs. Being able to register a directory as a dependency would probably be good enough, although it would mean rebuilding more frequently than necessary because someone could add/change a file in a directory but with a different file extension and trigger a rebuild. For example, if our glob is only looking for Ultimately I think being able to register files or directories but not globs would still be good enough for us. I wonder though if maybe it could be the job of postcss-loader to translate a glob into a set of files and directories instead, so from a PostCSS plugin author's perspective they still only need to be concerned with globs? Again though we can work with files + directories, it would just mean parsing the glob internally in Tailwind and turning that into a list of files and directories to register. Our workarounds for all of this currently is to run our own chokidar watcher and create a file in |
Glob support won't solve this problem, on the contrary, it degrades performance, because when you will evaluate glob, you will do a lot of fs calls, after you passed glob dependency for us, we will need to do it too on own side, so on each change you and we will need to a lot of fs calls. You shouldn't be afraid to pass
Yes we can do it, but as I said earlier this is not standardized and this greatly reduces performance (a lot of fs calls), and you already know this set of files, why not just give them to us. As I see it from my side:
Pros of this solution - bundle agnostic, built-in CLI (for example What can be The main problem is that we don't know which files require to rebuild files, and glob doesn't solve it, because we still need to look at whole directory here and do a lot of extra jobs, it is not solve the problem, patterns can be very complex... Only Other idea - you can get modified files using |
What do you think about these changes? Standard file dependency: { type: 'dependency', plugin, parent, file } Directory: { type: 'dependency', plugin, parent, dir }
// For compatibility we can add `file` prop as well { …, file, dir }
// We can use `type: 'dirDependency'` if we want a different type,
// but 'context-dependency' is not very clear name Missed: { type: 'dependency', plugin, parent, file, missed: true } “Build” dependency: { type: 'dependency', plugin, parent, file, invalidateAll: true } |
It is more misleading than we have, Why we need In most time developer need only Anyway I think using Also watching const watcher = chokidar.watch('file, dir, glob, or array', {
ignored: /(^|[\/\\])\../, // ignore dotfiles
persistent: true
});
watcher
.on('add', path => log(`File ${path} has been added`))
.on('change', path => log(`File ${path} has been changed`))
.on('unlink', path => log(`File ${path} has been removed`));
// More possible events.
watcher
.on('addDir', path => log(`Directory ${path} has been added`))
.on('unlinkDir', path => log(`Directory ${path} has been removed`)) So having |
Hey everyone, wanted to pitch in here on behalf of Tailwind CSS also. "Build" dependencies: As I understand it these are dependencies that invalidate the entire build. I agree with @alexander-akait that it is very rare that you would need this in a PostCSS plugin, and this is not something that we would require for Tailwind CSS. I am also concerned that not all bundlers could support this type of dependency currently. For example could this be supported by "Missing" dependencies: Again this is probably rare and not something that we would require. However, we could potentially utilise missing dependencies to optimise rebuilds. Again though, my concern is whether all bundlers understand the concept of "missing" dependencies. Parcel does via I think that it should be a priority to ensure that all tools/bundlers are able to support any new messages. For that reason I would like to propose that (at least initially) we standardise a way to register directory dependencies only, as a new type and/or a new property: { type: 'dependency', directory: '/my-project' }
// or
{ type: 'directory-dependency', directory: '/my-project' } Personally I think a new type is a better approach as it maintains the current situation of one dependency per message. With the first example you would be able to specify a file and a directory in the same message which feels a bit weird to me: { type: 'dependency', directory: '/my-project', file: '/random-file.txt' } Alternatively, if "build" and "missing" dependencies are desirable then I think we need to verify that tools can comfortably support them. What do you think? |
Looks rollup have https://rollupjs.org/guide/en/#thisaddwatchfileid-string--void, i.e. |
I don't understand what this means. If some file depends on the contents another file, then changing the second file would make Parcel rebuild the first. There shouldn't be any global state that somehow invalidates "everything". (The only similar mechanism is for JS config files, where the actual returned config isn't necessarily deterministic. So this causes some files to always be rebuilt on one-shot builds, but this mechanism is inherently incompatible with watch mode).
I want to second that. With many globs in the project, every file change event needs to checked against every glob, so the time for rebuilds now also depends on the size of the project, instead of just on the size of changes.
At least the moment, Parcel doesn't have an API for listening on file creations inside of a specific directory apart from globs. (But this would certainly be better for performance than globs.) |
For debug. In a real system, it could be 20+ plugins.
File, where this dependency was found. For instance, CSS file with
Yeap. We need to support the old API. I like
Runners should support each message feature only if they can improve DX by this message. If you do not know what to do with
There is a good idea, that we should describe “how we will process something” instead of popular use case. What do you think about
AT least for me, it is clear. A dependency for the file, which was not created yet. |
I found |
OK. So the final design:
Does it look good? |
I think for string better to use |
OK. It will be more CSS-ish.
We need compatibility with old API |
My concern with this is that if some dependency types are not supported by certain runners then they become unusable. If I create a PostCSS plugin using a "build" dependency and Rollup (for example) doesn't support it then my plugin won't work correctly with that runner. If you want to add these other dependency types perhaps we need to standardize a way for runners to declare which dependency types they support? Plugins can use this information to, for example:
Here's an example: const postcss = require('postcss')
postcss([
function (root, result) {
let supportedDependencyTypes = result.opts.dependencyTypes || ['dependency']
if (supportedDependencyTypes.includes('missing-dependency')) {
result.messages.push({ type: 'missing-dependency', /* ... */ })
} else if (supportedDependencyTypes.includes('dir-dependency')) {
result.messages.push({ type: 'dir-dependency', /* ... */ })
} else {
throw Error('Runner does not support directory dependencies.')
}
return root
},
])
// runner adds the list of dependency types it supports here
.process('/* ... */', { from: undefined, dependencyTypes: ['dependency', 'dir-dependency'] })
.then((result) => {
/* ... */
}) |
@bradlc I think it will be completely OK if the runner without Some messages could use advanced optimizations inside some specific bundler. Another bundler may have unique architecture and messages that will be relevant only for this architecture and can’t be used on different runners. |
I don't really agree with this. If I register a dependency as a build dependency then I expect it to behave like a build dependency. If it doesn't and just silently behaves like a normal dependency then that could lead to unexpected behaviour and bugs. |
It could be a runner, who does not have caches at all and always build everything from the scratch. |
Hey @ai, would love to keep the discussion going on this, with a view to getting something finalised and documented. How would you feel about initially adding documentation only for a new
Other types could be added in future but this is certainly the one that will be most useful for plugin authors, and enable new use-cases. Additionally, I still believe that runners should have a way to declare to plugins which message types they support. This will allow plugins to make informed decisions about which messages to use, or throw an error when a required message type is not supported. This is critical for developer experience. For example, we would love to use directory dependencies within the Tailwind CSS plugin to enable robust support for our JIT engine across all runners. We currently use workarounds for this, with varying levels of success. Ideally we would be able to do something like this: if (runnerSupportsDirDependencies) {
// use dir dependencies 👍
} else {
// use old workaround method, which still works in most cases 👍
} This would allow us to migrate to using directory dependencies in a seamless way that ensures that our plugin still works even with runners that have not yet implemented support for the new message type. |
Yeap, let’s add |
Dir dependency was added to plugin and runner guidelines |
We added optional |
result.messages.push({ type: 'dependency', file })
is a special API for plugin developers to tell about used files to PostCSS runner. The most popular way to use it is a watch mode.But seems like we need to improve this message:
postcss-mixins
accept glob pattern.missing
,build
,context
and regular dependencies. I do not like separated types likebuild-dependency
. I think it will be better to use{ type: 'dependency', build: true }
Questions:
The text was updated successfully, but these errors were encountered: