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

versionGroups not working with multiple elements #43

Closed
JasonGore opened this issue Sep 10, 2020 · 5 comments
Closed

versionGroups not working with multiple elements #43

JasonGore opened this issue Sep 10, 2020 · 5 comments

Comments

@JasonGore
Copy link

Description

I'm not sure if I'm using this feature correctly, but versionGroups is an array type which seems to imply it supports multiple entries. It works fine when I have one entry, but as soon as I add a second entry, it actually seems to break BOTH entries.

  1. Clone https://github.com/JasonGore/repro-syncpack
  2. yarn
  3. yarn repro

Expected:

No syncpack errors.

Actual:

Syncpack seems to ignore versionGroups and errors:

λ yarn repro
yarn run v1.22.4
$ syncpack list-mismatches --prod --dev
✕ fs-extra
- 9.0.1 in dependencies of syncpack-repro-a
- 9.0.1 in dependencies of syncpack-repro-c
- 9.0.1 in dependencies of syncpack-repro-a
- 8.0.1 in dependencies of syncpack-repro-b
- 9.0.1 in dependencies of syncpack-repro-c
✕ node-fetch
- 2.6.1 in dependencies of syncpack-repro-a
- 2.6.1 in dependencies of syncpack-repro-c
- 2.6.1 in dependencies of syncpack-repro-a
- 2.4.1 in dependencies of syncpack-repro-b
- 2.6.1 in dependencies of syncpack-repro-c
✕ uuid
- 6.0.0 in dependencies of syncpack-repro-a
- 8.3.0 in dependencies of syncpack-repro-c
- 6.0.0 in dependencies of syncpack-repro-a
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

If you just delete the second entry versionGroups in package.json from:

    "versionGroups": [
      {
        "dependencies": ["fs-extra", "node-fetch"],
        "packages": ["syncpack-repro-b"]
      },
      {
        "dependencies": ["uuid"],
        "packages": ["syncpack-repro-c"]
      }
    ]

to:

    "versionGroups": [
      {
        "dependencies": ["fs-extra", "node-fetch"],
        "packages": ["syncpack-repro-b"]
      }
    ]

Then you actually see some of the errors resolving:

λ yarn repro
yarn run v1.22.4
$ syncpack list-mismatches --prod --dev
✕ uuid
- 6.0.0 in dependencies of syncpack-repro-a
- 8.3.0 in dependencies of syncpack-repro-c
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I would expect both entries to work based on the type, but beyond that, it actually seems like the second entry breaks both entries.

@JamieMason
Copy link
Owner

Thanks @JasonGore, I will take a look.

@JamieMason
Copy link
Owner

JamieMason commented Sep 16, 2020

Had a look last night and I'm not too far off a fix, it'll take a me a few days mainly due to availability. One issue in the repro is that there are no sources, source is an empty array in the config and there are none passed via CLI options. This has uncovered another issue I should fix around falling back to the defaults.

The WIP is on the dev branch

@JamieMason
Copy link
Owner

Released in 5.6.10 🤞

Thanks for the handy repro, I have converted it into a test to catch any regressions:

describe('issue 43', () => {
it('should return no mismatches', () => {
const config: SyncpackConfig = {
...DEFAULT_CONFIG,
versionGroups: [
{ dependencies: ['fs-extra', 'node-fetch'], packages: ['syncpack-repro-b'] },
{ dependencies: ['uuid'], packages: ['syncpack-repro-c'] },
],
};
const iterator = getMismatchedDependencies(
[
mockWrapper({
name: 'syncpack-repro-a',
dependencies: { 'node-fetch': '2.6.1', 'fs-extra': '9.0.1', uuid: '6.0.0' },
}),
mockWrapper({
name: 'syncpack-repro-b',
dependencies: { 'node-fetch': '2.4.1', 'fs-extra': '8.0.1' },
}),
mockWrapper({
name: 'syncpack-repro-c',
dependencies: { 'node-fetch': '2.6.1', 'fs-extra': '9.0.1', uuid: '8.3.0' },
}),
],
config,
);
expect(Array.from(iterator)).toEqual([]);
});
});

Please let me know if you run into any more problems, thanks.

@JasonGore
Copy link
Author

It worked perfectly for my actual use case, which has 12 entries. Thanks for the quick fix!

@JamieMason
Copy link
Owner

That's great. You're welcome

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

No branches or pull requests

2 participants