Skip to content

Angular: Add versioned @types/node to packages installed during storybook init#34192

Open
Copilot wants to merge 3 commits into
nextfrom
copilot/fix-type-error-angular-projects
Open

Angular: Add versioned @types/node to packages installed during storybook init#34192
Copilot wants to merge 3 commits into
nextfrom
copilot/fix-type-error-angular-projects

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 18, 2026

Fresh Angular projects initialized with pnpm miss @types/node (due to pnpm's strict dependency isolation), causing a Webpack/TypeScript error at startup: Cannot find type definition file for 'node'.

Changes

  • generators/ANGULAR/index.ts: Detect the user's Node.js major version at init time and add the matching @types/node@^{majorVersion} (e.g. @types/node@^22 on Node.js 22) to extraPackages returned from the Angular generator's configure function, ensuring it is installed as a dev dependency during storybook init

For npm/yarn users where @types/node is already present transitively, the explicit versioned install is a no-op. For pnpm users it resolves the missing type definition with types that match their actual Node.js runtime.

View original Slack conversation


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Summary by CodeRabbit

  • Bug Fixes
    • Improved automatic dependency management for Node.js type definitions when generating Angular projects, ensuring compatibility with the current Node.js version.

Co-authored-by: valentinpalkovic <5889929+valentinpalkovic@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix type error in newly initialised Angular projects fix(angular): add @types/node to packages installed during storybook init Mar 18, 2026
Copilot AI requested a review from valentinpalkovic March 18, 2026 08:46
return {
extraPackages: [
...extraAngularDeps,
'@types/node',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@copilot . I think this is too simple. We need to detect the user's Node.js version and install @types/node based on the user's major Node.js version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in f4810a1 — we now read process.versions.node at init time, extract the major version, and install @types/node@^{majorVersion} (e.g. @types/node@^22 on Node.js 22).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we support other runtimes like deno or bun ? I'm not sure that process.versions.node would always be populated.
This can result in NaN

Copy link
Copy Markdown
Contributor

@valentinpalkovic valentinpalkovic Mar 19, 2026

Choose a reason for hiding this comment

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

I know that bun sets process.versions.node for sure! Deno as well: https://docs.deno.com/api/node/process/~/Process.versions. Officially, we don't have Deno support, and our bun runtime support is experimental

…rsion

Co-authored-by: valentinpalkovic <5889929+valentinpalkovic@users.noreply.github.com>
Copilot AI changed the title fix(angular): add @types/node to packages installed during storybook init fix(angular): add versioned @types/node to packages installed during storybook init Mar 18, 2026
Copilot AI requested a review from valentinpalkovic March 18, 2026 08:51
@valentinpalkovic valentinpalkovic changed the title fix(angular): add versioned @types/node to packages installed during storybook init Angular: Add versioned @types/node to packages installed during storybook init Mar 18, 2026
@valentinpalkovic valentinpalkovic added bug ci:normal Run our default set of CI jobs (choose this for most PRs). labels Mar 18, 2026
@valentinpalkovic valentinpalkovic marked this pull request as ready for review March 18, 2026 09:01
@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Mar 18, 2026

View your CI Pipeline Execution ↗ for commit f4810a1

Command Status Duration Result
nx run-many -t compile,check,knip,test,lint,fmt... ✅ Succeeded 8m 13s View ↗

☁️ Nx Cloud last updated this comment at 2026-04-14 08:58:57 UTC

@github-actions
Copy link
Copy Markdown
Contributor

Fails
🚫 PR description is missing the mandatory "#### Manual testing" section. Please add it so that reviewers know how to manually test your changes.

Generated by 🚫 dangerJS against f4810a1

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c482704e-75e6-4501-a821-c3f5b2303442

📥 Commits

Reviewing files that changed from the base of the PR and between e3c5772 and f4810a1.

📒 Files selected for processing (1)
  • code/lib/create-storybook/src/generators/ANGULAR/index.ts

📝 Walkthrough

Walkthrough

The change introduces a runtime calculation of the Node.js major version extracted from process.versions.node and appends a corresponding @types/node@^<major> entry to the extraPackages array for Angular Storybook generator configurations.

Changes

Cohort / File(s) Summary
Angular Generator
code/lib/create-storybook/src/generators/ANGULAR/index.ts
Added Node.js major version calculation and injected version-pinned @types/node package into extraPackages array.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@Sidnioulz Sidnioulz left a comment

Choose a reason for hiding this comment

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

The currently running node version has nothing to do with the type declarations consumed by Storybook or Angular. We should install the right version. As far as we're concerned, it's the minimal supported version we consume.

Comment thread code/lib/create-storybook/src/generators/ANGULAR/index.ts
@valentinpalkovic
Copy link
Copy Markdown
Contributor

valentinpalkovic commented Mar 19, 2026

The currently running node version has nothing to do with the type declarations consumed by Storybook or Angular. We should install the right version. As far as we're concerned, it's the minimal supported version we consume.

@Sidnioulz The runtime doesn't distinguish between node_modules and user code. One Node.js runtime is processing both. Installing @types/node globally in the user's project with a version different from the version that the project uses doesn't make much sense to me. Lastly, the user may also write Node.js code, and the types should match their version. I understand you're confused, though. If we use API's which are incompatible with the user's Node.js version, a type error would occur. But this is great, isn't it? Users should be notified if we rely on Node.js APIs that the project's Node.js version doesn't support. They should reach out via a GitHub issue to let us know that we don't support the API for a certain Node.js version.

@Sidnioulz
Copy link
Copy Markdown
Contributor

The currently running node version has nothing to do with the type declarations consumed by Storybook or Angular. We should install the right version. As far as we're concerned, it's the minimal supported version we consume.

@Sidnioulz The runtime doesn't distinguish between node_modules and user code. One Node.js runtime is processing both. Installing @types/node globally in the user's project with a version different from the version that the project uses doesn't make much sense to me. Lastly, the user may also write Node.js code, and the types should match their version. I understand you're confused, though. If we use API's which are incompatible with the user's Node.js version, a type error would occur. But this is great, isn't it? Users should be notified if we rely on Node.js APIs that the project's Node.js version doesn't support. They should reach out via a GitHub issue to let us know that we don't support the API for a certain Node.js version.

My point was that a user setting up SB might be running Node 25, but their colleagues might be on 22.22.1. Suddenly they have types that might not match their runtime because some breaking changes happened because the person running the init has a more recent version.

In contrast, if we always installed 22.19, we'd know we're not using any Node 25 exclusive APIs because that's our minimal required version. And we'd still run into issues if the project needs lower versions than us.

But I'm now suspecting what you want to ensure is that users with a lower runtime version than our minimal version don't get forced into an upgrade? Would we then want to

  • install that lower version and brace ourselves for absence of errors when they run SB?
  • warn ahead of time of incompatible versions?

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

Labels

angular bug ci:normal Run our default set of CI jobs (choose this for most PRs). cli needs triage Stale

Projects

Status: Empathy Queue (prioritized)

Development

Successfully merging this pull request may close these issues.

4 participants