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

Option to disable package.json special-casing #172

Closed
cspotcode opened this issue Feb 22, 2019 · 12 comments
Closed

Option to disable package.json special-casing #172

cspotcode opened this issue Feb 22, 2019 · 12 comments

Comments

@cspotcode
Copy link

In rare cases, I want to use cosmiconfig in a generic way, to load from a user-specified config file. I want my code to behave identically regardless of the filename specified by the user, so I need to disable cosmiconfig's special-casing of package.json.

One hack is to ask for ./package.json instead of package.json. However, I think a better approach is for Explorer to have a property packageJsonName which is a string or array of strings listing filenames that should be treated as package.json instead of a normal config file.

This property can be modified by people like me who need to treat package.json like any other config file.

@davidtheclark
Copy link
Collaborator

@cspotcode package.json is a very loaded file name. You really want it to be treated like any other configuration file? I don't think I understand your use case. Could you please explain more?

@cspotcode
Copy link
Author

cspotcode commented Feb 25, 2019

There are 2 possible use-cases:

a. Treating another file, perhaps manifest.json, the same way as package.json. This might be useful when developing Chrome extensions.

b. Using cosmiconfig to discover the location of a config file without actually loading it.

I started investigating cosmiconfig as a replacement for findup-sync in Mocha's new v6. findup-sync is a big contributor to slow mocha startup time, exacerbated by the fact that mocha needs to spawn an extra process internally. (the process spawning is necessary for good reasons) cosmiconfig loads much faster.

However, I had to disable cosmiconfig's config loading, because Mocha already has specific ways it loads config files. Setting loaders: {'.js': noopFn, '.json': noopFn, //...} does the trick. Unfortunately, there's no override for how package.json files are loaded. And since I need mocha to use its existing logic to avoid breaking changes and preserve its diagnostic messages, I need to disable cosmiconfig's special-casing behavior. (I did this with packageProp: '__proto__' but that's really hacky)

An alternative is exposing an API to locate files but not load them. This could be an iterator, compatible with for-of. Keep calling it and it keeps returning the next candidate config file.


I realize my use-case might not be very common, but the implementation in #173 is very clean. Instead of a magic, hardcoded string, it's a config value. All the logic is still just as simple, and default behavior is identical to today.

Your README says the search process is highly customizable; this change is in the same spirit.

@davidtheclark
Copy link
Collaborator

b. Using cosmiconfig to discover the location of a config file without actually loading it.

This seems like a nice feature on its own, independent of any package.json-specific use cases.

there's no override for how package.json files are loaded

I see: you can turn it off, but not create a special loader. Would you still want a special package.json loader if you had the new feature described above — find the file but don't load it?

@cspotcode
Copy link
Author

Would you still want a special package.json loader if you had the new feature described above — find the file but don't load it?

Ideally yes, but it's not that important. E.g if someone runs mocha --package ./manifest.json then it would be convenient for cosmiconfig to take care of discovering a ./manifest.json and loading the mocha property. But calling require('./package.json').mocha or require(opts.package).mocha are both easy to handle within mocha, so it's not a big deal.

I'll open a new issue to talk about an API for locating and not loading config files.

@davidtheclark
Copy link
Collaborator

My proposal for this is that we extend the loaders option so keys can be file basenames, as well as just extensions. So you could have a loaders value like this:

{
  'package.json': specialLoader,
  'manifest.json': specialLoader,
  '.json': someOtherJsonLoader,
  ..
}

When we look for a loader for a file, then, we'd first check if the file's basename has a corresponding loader; if not, we'll check the extension for a loader.

How does that sound to others?

@olsonpm
Copy link
Contributor

olsonpm commented Mar 9, 2019

sounds easy enough - my only concern is that this feature get used, so we can tinker/hack until the mocha team sees real value in swapping before investing too much time. If mocha ends up using this library that'd be pretty cool.

I'm also not opposed to expanding the api to attract more potential users if that's the intent, I just would want that to be clear up front to prevent another case of the virtual fs PR.

@cspotcode
Copy link
Author

Is this proposal compatible with config files like .foorc? Or will there be ambiguity with file extension handling?

Will the package.json-loader be exposed like the others? For example: {'manifest.json': cosmiconfig.loadPackageJson}

Ideally this feature does not add gotchas or complexity for pre-existing users. 9 times out of 10, a user will not want to use this feature, and the API should remain simple in those situations. This feature should only make itself known to those who really want to use it.

#173 seems like an easier approach. It keeps all the existing logic identical, simply allowing a previously-hardcoded string to be configurable.


If I end up proposing find-up for mocha (#176 (comment)) then this might be unnecessary.

@davidtheclark
Copy link
Collaborator

Is this proposal compatible with config files like .foorc? Or will there be ambiguity with file extension handling?

You'd have to handle extensionless files the way you do now, with a noExt key.

Will the package.json-loader be exposed like the others? For example: {'manifest.json': cosmiconfig.loadPackageJson}

No, because it doesn't fit the signature of all the other loaders: it needs to receive a property name as well as the filepath and contents. It should be a straightforward-enough custom loader in edge cases.

I'm not on board with packageFilenames right now for two reasons:

  • It's not clear what the name packageFilenames means, what the "package" file abstraction really means in this context; and I think that reveals an API ambiguity that could prove troublesome.
  • It does not solve the problem of potentially providing a custom loader for package.json.

@cspotcode
Copy link
Author

No, because it doesn't fit the signature of all the other loaders: it needs to receive a property name as well as the filepath and contents. It should be a straightforward-enough custom loader in edge cases.

It can be exposed as a factory, right? {'manifest.json': cosmiconfig.loadPackageJson('config.property')}

@cspotcode
Copy link
Author

I ended up using find-up in mocha, so I'm no longer investigating cosmiconfig. I appreciate everyone taking the time to discuss. mochajs/mocha#3830

@davidtheclark
Copy link
Collaborator

👍 I'll leave this issue open to see if others want this feature.

@davidtheclark
Copy link
Collaborator

Closing as stale.

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

No branches or pull requests

3 participants