Skip to content
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

feat(loader): scan for config keys files #210

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ineshbose
Copy link

attempts to resolve #137

file tree example:

tailwind.config.js
tailwind.theme.config.js
tailwind.theme.extend.config.js
.config/tailwind.config.js
.config/tailwind.js
.config/tailwind/theme.js
.config/tailwind.theme.js
.config/tailwind.theme.config.js
.config/tailwind/theme.config.js
.config/tailwind/theme/extend.js

@ineshbose
Copy link
Author

So I've added some code, but I realise maybe it doesn't belong properly here.

You're also right pi0 that glob won't be too efficient as I understand that in NodeJS implementations we scandir and then do picomatch. In any case, I was wondering that the first call to resolve config location, we get all files that contain the config name as a fragment (either with . or /) - this is also useful in cases where name.config.js, .config/name.js and .config/name.config.js all exist together - then we should also ideally sort the found paths based on priority, for eg, parent configs should be higher priority while merging (./name.config.js > .config/name.js > .config/name/index.js, etc - kind of based on path length I suppose).

@ineshbose
Copy link
Author

I think with this, it may also be possible to resolve #157 and #206 (based on my understanding)

@pi0
Copy link
Member

pi0 commented Dec 13, 2024

Thanks for PR. Some changes:

  • I think we should this convention opt-in
  • We should use readDir that reads exactly one level (not nested), we might support more levels but I don't want to go into a path that we later can't come back. So only .config/tailwind/theme.[ext] (also only for .config/ patterns as part of config-dir proposal.

@ineshbose
Copy link
Author

Thank you for the feedback (and patience as I have been context-switching as would you)!

  • I think we should this convention opt-in

Fair enough - I understand this may be expensive!

  • We should use readDir that reads exactly one level (not nested), we might support more levels but I don't want to go into a path that we later can't come back.

Now that you point that out, I realise that this feature is essentially the reverse of exposeConfig in @nuxtjs/tailwindcss 😄

Over there, we have defaulted to 4 level-deep for generating templates of the config object. So based on my understanding of GlobOptions from tinyglobby, I have added deep: 1 as default, while I feel that this should remain configurable as one-level of config splitting may provide low-benefit to some config structures.

So only .config/tailwind/theme.[ext] (also only for .config/ patterns as part of config-dir proposal.

I do agree with this point as well, but I would believe c12 to not be bias-ed towards any code-organisation practice by limiting features to the .config dir (while I am onboard with the proposal as well, but I'd rather not be restricting existing way since support isn't 100% either). If anything, having split config files on the rootDir is all-the-more reason for devs to consider the .config/ dir, so we would encourage it in that case. Either way - I am not very fussed and will respect whatever call you make!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support nested config splitting in .config/ dir
2 participants