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

Make syncpack configurable from the 'config' option in package.json #86

Closed
bombillazo opened this issue Jul 10, 2022 · 7 comments
Closed

Comments

@bombillazo
Copy link

bombillazo commented Jul 10, 2022

Description

Currently, when configuring syncpack from package.json, it is only configurable with the syncpack option directly in the root of the package.json file. For compatibility with the config option, it would be great if syncpack also checked for the config inside the config option.

Suggested Solution

Have syncpack check for the configuration inside config:

{
  ...
  "config" : {
    "syncpack" : {
      // syncpack configuration
    }
}
@bombillazo bombillazo changed the title make syncpack configurable from the config option in package.json Make syncpack configurable from the 'config' option in package.json Jul 10, 2022
@JamieMason
Copy link
Owner

Hey Hector, this is all handled by https://github.com/davidtheclark/cosmiconfig – not sure yet but maybe it's some option we need to pass or it's something they might add as a new feature on request.

@bombillazo
Copy link
Author

bombillazo commented Jul 11, 2022

Hey, thanks for the response, here's the prop you're looking for: packageProp :
['config', 'syncpack']

Looks like it only allows one value for package.json, it would've been nice if multiple values were could be configured.

@JamieMason
Copy link
Owner

Had a quick look at this just now @bombillazo, what it would need is for syncpack to run cosmiconfig a 2nd time if result is null the first time.

const result = configPath ? client.load(configPath) : client.search();

I'll lower the priority on this rather than close it, as I can see why it would be desirable. But a relative minority of users would use it, while all users which don't use the default config file location would get a performance penalty.

Thanks for raising, let's look again in a while.

@bombillazo
Copy link
Author

bombillazo commented Oct 28, 2022

Sure, one recommendation I could give is to directly read the package.json file and see which keys exist before running cosmiconfig:

var pjson = require('./package.json');
const hasConfigOption = !!pjson.config.syncpack

The performance penalty for this is minimal, especially compared to rerunning cosmiconfig.

@JamieMason
Copy link
Owner

Yeah, good shout 👍🏻

@JamieMason
Copy link
Owner

Released in 8.3.8, thanks a lot.

@bombillazo
Copy link
Author

Thank you!

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

2 participants