Skip to content

Conversation

@vighnesh153
Copy link
Contributor

Description

Moved the @types/node from dependencies to devDependencies

Motivation and Context

I am using a mono repo and all my packages use a single version of @types/node. Since, the @types/node in this @commitlint/load package is very old and doesn't match the version in my projects, it creates a different node_modules directory for all my sub-projects because of this version mis-match.

How Has This Been Tested?

Build succeeds

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
@vighnesh153
Copy link
Contributor Author

@escapedcat Drawing your attention. 🙂 Let me know if you see any concerns.

@escapedcat
Copy link
Member

No, thanks, should be fine. Was just waiting for the green checks.

@escapedcat escapedcat merged commit c0b32fe into conventional-changelog:master Dec 27, 2022
@SimenB
Copy link

SimenB commented Jan 6, 2023

This leads to peer dependency warnings.

➤ YN0002: │ @commitlint/load@npm:17.4.0 doesn't provide @types/node (p6db7e), requested by cosmiconfig-typescript-loader ➤ YN0002: │ @commitlint/load@npm:17.4.0 doesn't provide @types/node (p73968), requested by ts-node 

Optional peer dependency with * range or something is probably a good idea (or just keep it as a regular dependency but with * as version so consuming projects can dedupe to solve OP's issue)

SimenB added a commit to jest-community/eslint-plugin-jest that referenced this pull request Jan 6, 2023
@escapedcat
Copy link
Member

Ah damn, optional deps for ts support are a never ending story.
@vighnesh153 would you mind adding it again but using * as suggested? As long as as we can't add cosmiconfig-typescript-loader optionally we need to keep these deps.
Thanks @SimenB !

@vighnesh153
Copy link
Contributor Author

@escapedcat Unfortunately, I deleted the forked repository and didn't have access to push to this pull request any more. Have created another pull request for addressing this issue:

#3490

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

Labels

None yet

3 participants