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

chore: bug reports to include @vitest/* packages versions #2725

Merged
merged 1 commit into from
Jan 23, 2023

Conversation

AriPerkkio
Copy link
Member

Include @vitest/* packages in envinfo. Helps narrowing down issues like #2716 where users have different versions of vitest and other @vitest/* packages running.

@AriPerkkio
Copy link
Member Author

The checklist could also include following:

- label: Check that the versions of `vitest` and other `@vitest/*` packages are identical
  required: true

Any thoughts?

@sheremet-va
Copy link
Member

sheremet-va commented Jan 21, 2023

The checklist could also include following:

- label: Check that the versions of `vitest` and other `@vitest/*` packages are identical
  required: true

Any thoughts?

Maybe instead we should change peerDependencies from * to workspace:* (gets replaced with the current version, when published)?

@AriPerkkio
Copy link
Member Author

Yes, peerDependencies is the correct way to solve these conflicting versions. Looks like the coverage packages are not included as optional peer dependencies in vitest package's package.json at all.

Should the @vitest/* packages also have vitest as required peer dependency? Currently they list vitest as regular production dependency instead.

@sheremet-va
Copy link
Member

Should the @vitest/* packages also have vitest as required peer dependency? Currently they list vitest as regular production dependency instead.

I think only coverage packages depend on Vitest. Others can be used without Vitest at all (except for UI maybe).

@AriPerkkio
Copy link
Member Author

AriPerkkio commented Jan 23, 2023

I think the peer dependencies should be set as following, using 0.27.2 version as example:

{
  "name": "vitest",
  "version": "0.27.2",
  "peerDependencies": {
    "@vitest/coverage-c8": "0.27.2",
    "@vitest/coverage-istanbul": "0.27.2"
  },
  "peerDependenciesMeta": {
    "@vitest/coverage-c8": {
      "optional": true
    },
    "@vitest/coverage-istanbul": {
      "optional": true
    }
  }
}
{
  "name": "@vitest/coverage-c8",  "//": "Same for @vitest/coverage-istanbul package",
  "version": "0.27.2",
  "dependencies": {
    "//": "No vitest here. This package should only be used in a project that has vitest as dependency. Do not bring it with this package."
  },
  "peerDependencies": {
    "vitest": "0.27.2"
  },
  "peerDependenciesMeta": {
    "vitest": {
      "optional": false, "//": "Default value so can be omitted"
    }
  }
}

Currently the @vitest/coverage-x packages declare vitest as production dependency. They should not. Currently it's possible to cause conflicts where @vitest/coverage-x packages bring in their own vitest instead of utilizing the peer dependency:

$ npm i -D -E vitest@0.26.0
$ npm i -D -E @vitest/coverage-istanbul@0.27.2
$ npm ls vitest
repro@1.0.0 /Users/x/y/repro
├─┬ @vitest/coverage-istanbul@0.27.2
│ └── vitest@0.27.2
└── vitest@0.26.0

# Check from file system as well:
$ grep version node_modules/vitest/package.json 
  "version": "0.26.0",
$ grep version node_modules/@vitest/coverage-istanbul/package.json
  "version": "0.27.2",
$ grep version node_modules/@vitest/coverage-istanbul/node_modules/vitest/package.json
  "version": "0.27.2",

@sheremet-va
Copy link
Member

I think the peer dependencies should be set as following, using 0.27.2 version as example:

I don't think versions should be hardcoded, * is enough. Also vitest is not an optional peer dependency for coverage providers.

@AriPerkkio
Copy link
Member Author

AriPerkkio commented Jan 23, 2023

Also vitest is not an optional peer dependency for coverage providers.

Yes exactly, this is what the peerDependenciesMeta.vitest.optional: false above means.

I don't think versions should be hardcoded, * is enough.

Using * is fine if the packages are indeed guaranteed to work with mixed versions. I think I'll just need to be more careful in future and not introduce breaking changes between vitest and @vitest/* packages like below:

If the peerDependencies was used to enforce versions these conflicts would come up when updating the packages. But hardcoding the versions to exact version on every release would not be a solution, as those changes of peer dependency requirements would yet be another breaking change. Maybe let's leave them as is for now and just update this issue template.

@sheremet-va sheremet-va merged commit 2a7f6f8 into vitest-dev:main Jan 23, 2023
@sheremet-va
Copy link
Member

sheremet-va commented Jan 23, 2023

@AriPerkkio feel free to open PR with the versions then. I don't think we need to mention the same versions in an issue form, since it's not usually a requirement 🤔

@AriPerkkio AriPerkkio deleted the chore/issue-template-pkgs branch January 23, 2023 15:59
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.

3 participants