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

fix(types): don't pin webpack types to exact version #23531

Merged
merged 2 commits into from Oct 10, 2023

Conversation

rchl
Copy link

@rchl rchl commented Oct 4, 2023

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Since the current convention in @nuxt/types is to pin all dependencies to exact versions, I'm pretty sure this is not an acceptable solution but I wanted to raise an issue with the current approach.

The @types/webpack is pinned to 4.41.33. Since 4.41.34 is now the latest version, if I do yarn upgrade in my project, this will introduce both 4.41.33 and 4.41.34 and then the shim that extends webpack from @nuxt/types/shims.d.ts will stop working. A code like:

import '@nuxt/types'
import { MultiStats, AssetInfo } from 'webpack'

should show that MultiStats that the shim is supposed to introduce is not present.

It works if only 4.41.33 is installed in the project.

I'm not sure what to do with this problem. Clearly there was a reason for going with exact versions but as evident here, this is also not a good solution. Ideally a range should be allowed and as long as packages follow semver, it should not cause issues (but clearly did in the past).

@danielroe

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@stackblitz
Copy link

stackblitz bot commented Oct 4, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@codecov-commenter
Copy link

codecov-commenter commented Oct 4, 2023

Codecov Report

All modified lines are covered by tests βœ…

Comparison is base (6132947) 66.13% compared to head (4357799) 66.13%.

Additional details and impacted files
@@           Coverage Diff           @@
##              2.x   #23531   +/-   ##
=======================================
  Coverage   66.13%   66.13%           
=======================================
  Files          93       93           
  Lines        4125     4125           
  Branches     1171     1171           
=======================================
  Hits         2728     2728           
  Misses       1130     1130           
  Partials      267      267           
Flag Coverage Ξ”
unittests 66.13% <ΓΈ> (ΓΈ)

Flags with carried forward coverage won't be shown. Click here to find out more.

β˜” View full report in Codecov by Sentry.
πŸ“’ Have feedback on the report? Share it here.

@rchl
Copy link
Author

rchl commented Oct 4, 2023

CI here also started failing because of that issue.

@rchl rchl mentioned this pull request Oct 4, 2023
8 tasks
@@ -23,7 +23,7 @@
"@types/pug": "2.0.6",
"@types/serve-static": "1.15.2",
"@types/terser-webpack-plugin": "4.2.1",
"@types/webpack": "4.41.33",
"@types/webpack": "^4.41.33",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe?

Suggested change
"@types/webpack": "^4.41.33",
"@types/webpack": "~4.41.33",

Copy link
Author

Choose a reason for hiding this comment

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

Should be fine for now if you prefer that but could cause a similar issue in the future.

For reference, there seemed to be an issue with @types/webpack-hot-middleware before that broke types in a patch release (#18827) but I'm not aware of there being any issues with @types/webpack.

Copy link
Author

@rchl rchl Oct 10, 2023

Choose a reason for hiding this comment

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

@danielroe so would you prefer to use ~? I'm a bit skeptical because then it's guaranteed to break once webpack types update the minor version. I'd rather trust and hope that they don't break types in minor revision.

@danielroe danielroe merged commit 269eadd into nuxt:2.x Oct 10, 2023
12 of 13 checks passed
@github-actions github-actions bot mentioned this pull request Oct 10, 2023
@rchl rchl deleted the fix/webpack-types branch October 10, 2023 11: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.

None yet

3 participants