Ensure content globs defined in @config files are relative to that file#14314
Conversation
philipp-spiess
requested review from
RobinMalfait,
adamwathan and
thecrypticace
| base: inputBasePath, // Globs are relative to the input.css file | ||
| sources: compiler.globs.map(({ base, pattern }) => ({ | ||
| // Globs are relative to the input.css file | ||
| base: base ? path.dirname(path.resolve(inputBasePath, base)) : inputBasePath, |
There was a problem hiding this comment.
This makes me wonder if we shouldn't call this something else? We can't resolve the base inside tailwindcss core (if we do, we need to make it aware of paths etc. which feels wrong if it doesn't do IO?) so instead, we have to translate the paths that are relative to the input CSS file here to the proper base dirname.
My proposal is to call it origin instead of base. That would also make it clearer why origin can be null (i.e it's origin is the input CSS file)
I went with base for consistency with the other places where we use globs.
Update: I went ahead and called it origin for now. Still curious what you think here, though!
There was a problem hiding this comment.
I like origin if it’s a full file name and not the directory, is that the case? The full file path feels more correct to me with that name (“this is the file this source came from”) if it doesn’t introduce any other problems.
There was a problem hiding this comment.
I like origin if it’s a full file name and not the directory, is that the case?
Yeah that's the case, it's basically just keeping track of whatever string is after @plugin or @config right now, it's only transformed if it's relative inside @imports (using the postcss plugin) but core doesn't do any other manipulation. So it's a file path that is relative to the input base path
philipp-spiess
force-pushed
the
fix/ensure-globs-in-config-are-relative-to-that-file
branch
from
9d1bc73 to
7065edf
Compare
|
I would almost rather pass |
| 'project-a/src/index.css': css` | ||
| @import 'tailwindcss/utilities'; | ||
| @source '../../project-b/src/**/*.js'; | ||
| @config '../tailwind.config.js'; |
There was a problem hiding this comment.
Is there any value in testing @source and @config here instead of just switching to @config?
There was a problem hiding this comment.
Hmmm. I was initially thinking no but since we do return origin of undefined for globs defined inside the css file, it does behave differently (and that is implemented in each client right now). I wanted to avoid having to split all tests again to support that though, perhaps we can at least include both cases in the same test (so it doesn't mean we double all our basic integration tests right away).
There was a problem hiding this comment.
Added back the @source tests
| sources: compiler.globs.map((pattern) => ({ | ||
| base: inputBasePath, // Globs are relative to the input.css file | ||
| sources: compiler.globs.map(({ origin, pattern }) => ({ | ||
| // Globs are relative to the input.css file |
There was a problem hiding this comment.
I know this comment was already there but I wonder if it's misleading to say input.css here since the file could be named anything? And origin comes from a config file, not a CSS file.
Maybe these should be more like:
Globs are relative to the CSS file or config file where they are specified
There was a problem hiding this comment.
I left it there because the origin is also relative to the input CSS file right now, but yeah this is confusing. Maybe:
Globs need to be relative to the CSS file or config file where they are specified. An eventual origin is relative to the CSS file.
But this is really confusing now 😬
There was a problem hiding this comment.
How about:
Ensure the glob is relative to the input CSS file or the config file where they are specified.
There was a problem hiding this comment.
Updated it to the latest one
Do you have an example of what you mean? I think |
Right and we don't want to call path.dirname in core. yeah okay I'm not gonna fight to hard for this — just thinking out loud. |
philipp-spiess
deleted the
fix/ensure-globs-in-config-are-relative-to-that-file
branch
3 participants
When you configure custom content globs inside an
@configfile, we want to tread these globs as being relative to that config file and not the CSS file that requires the content file. A config can be used by multiple CSS configs.