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: add dependenciesCustomPaths options #113

Closed

Conversation

LudovicSterlin
Copy link
Contributor

Description (What)

Add of dependenciesCustomPaths option to resolve #112

Justification (Why)

See #112

How Can This Be Tested?

One test has been added for list-mismatches and fix-mismatches. Let me know if you see other useful ones that I can add.

yarn install
yarn test

I could also put online a minimal version of the monorepo where I used this feature to see the result at bigger scale.

@JamieMason
Copy link
Owner

Thanks a lot for this @LudovicSterlin, I like the idea. This PR comes just as I've done a lot of refactoring so there are loads of conflicts now due to that. I'm happy to pick this up though to save you redoing a lot of work.

@LudovicSterlin
Copy link
Contributor Author

I can also try to rebase on master if you want ?

Comment on lines +59 to +72
function updateObjectNestedKeyFromPath(
obj: any,
nestedPropertyPath: string,
value: string | undefined,
) {
const properties = nestedPropertyPath.split('.');
const lastKeyIndex = properties.length - 1;
for (let i = 0; i < lastKeyIndex; ++i) {
const key = properties[i];
if (!(key in obj)) obj[key] = {};
obj = obj[key];
}
obj[properties[lastKeyIndex]] = value;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function should probably not be here but in a utils file somewhere, but I didn't know where to put it, so I let it here for now, waiting for instructions.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! yeah something like this will definitely be needed.

We have props('path.t.a.value', isNumber)(someObject) (tests here) for getting but nothing for setting. If you'd like to make this that would be great.

I'm using https://mobily.github.io/ts-belt/api/result a lot which I appreciate is a style of coding that may not be to everyone's taste, so if you don't want to that's fine too.

Thanks a lot for this PR and your work so far, it's unlucky timing that it came right when I was doing a huge refactor. Things will be settled again now.

@JamieMason
Copy link
Owner

I can also try to rebase on master if you want ?

Thanks a lot but I'd hold on as I"d like to refactor a few things in src/lib/get-context/get-config/index.ts which will hopefully make implementing this feature easier – it's not ideal how there is the ALL_DEPENDENCY_TYPES array but then the dependency type CLI options which have slightly different names.

It'd be good to normalise those aliases into paths (eg. "pnpmOverrides" -> "pnpm.overrides") so we can have an array of them, where custom ones just get added into the array for processing by syncpack down the chain:

["pnpm.overrides", "dependencies", "devDependencies", "directus:extension.host"]

Those paths can then be read by src/lib/get-context/get-package-json-files/get-patterns/props.spec.ts to get the values.

JamieMason added a commit that referenced this pull request Feb 14, 2023
BREAKING CHANGE:

1. The following options were replaced in syncpack@9.0.0:

  -p, --prod              include dependencies
  -d, --dev               include devDependencies
  -P, --peer              include peerDependencies
  -R, --resolutions       include resolutions (yarn)
  -o, --overrides         include overrides (npm)
  -O, --pnpmOverrides     include overrides (pnpm)
  -w, --workspace         include locally developed package versions

  Instead use the new --types option like so:

    --types dev,prod,peer

2. In .syncpackrc, the following options were replaced:

  "dev": true,
  "overrides": true,
  "peer": true,
  "pnpmOverrides": true,
  "prod": true,
  "resolutions": true,
  "workspace": true,

  Instead use the new dependencyTypes array like so:

    "dependencyTypes": ["dev", "prod", "peer"]

Closes #112
Closes #113

Co-authored-by: Ludovic Sterlin <ludovic.sterlin@hinfact.com>
JamieMason added a commit that referenced this pull request Feb 14, 2023
BREAKING CHANGE:

1. The following options were replaced in syncpack@9.0.0:

  -p, --prod              include dependencies
  -d, --dev               include devDependencies
  -P, --peer              include peerDependencies
  -R, --resolutions       include resolutions (yarn)
  -o, --overrides         include overrides (npm)
  -O, --pnpmOverrides     include overrides (pnpm)
  -w, --workspace         include locally developed package versions

  Instead use the new --types option like so:

    --types dev,prod,peer

2. In .syncpackrc, the following options were replaced:

  "dev": true,
  "overrides": true,
  "peer": true,
  "pnpmOverrides": true,
  "prod": true,
  "resolutions": true,
  "workspace": true,

  Instead use the new dependencyTypes array like so:

    "dependencyTypes": ["dev", "prod", "peer"]

Closes #112
Closes #113

Co-authored-by: Ludovic Sterlin <ludovic.sterlin@hinfact.com>
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.

Sync version in custom package.json properties
2 participants