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

@storybook/react types dependency @types/webpack-env conflicting with @types/node #9241

Closed
piyushkmr opened this issue Dec 24, 2019 · 4 comments · Fixed by DefinitelyTyped/DefinitelyTyped#41313 or #9365

Comments

@piyushkmr
Copy link

Describe the bug
@storybook/react types dependency @types/webpack-env conflicting with @types/node

node_modules/@types/webpack-env/index.d.ts:282:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'require' must be of type 'Require', but here has type 'NodeRequire'.

To Reproduce
Steps to reproduce the behavior:

  1. Setup a TS env
  2. Install @storybook/react
  3. Install @types/node
  4. Compile project

Screenshots
Screenshot 2019-12-24 at 4 27 34 PM

System:

System:
    OS: macOS 10.15.1
    CPU: (8) x64 Intel(R) Core(TM) i7-3635QM CPU @ 2.40GHz
  Binaries:
    Node: 10.16.0 - ~/.nvm/versions/node/v10.16.0/bin/node
    Yarn: 1.17.3 - /usr/local/bin/yarn
    npm: 6.9.0 - ~/.nvm/versions/node/v10.16.0/bin/npm
  Browsers:
    Chrome: 79.0.3945.88
    Safari: 13.0.3

Additional context
Manually editing @types/webpack-env solves this problem.
I tried adding "types" to tsconfig.json to limit type definition scope.
I am using storybook in monorepo setup using yarn.

@aqm1152
Copy link

aqm1152 commented Dec 26, 2019

Having the same exact issue with the same system. Also building a typescript project. The only way I was able to get my project to build was making skiLibCheck in tsconfig to true.

@aqm1152
Copy link

aqm1152 commented Dec 27, 2019

Having to revert @types/node to version 12.12.18, the project builds fine. would be nice to get a fix on this.

@andrewbranch
Copy link

Here’s the problem.

The package @types/node exists to describe the intrinsic environment that programs have when they run in node, including the type of globals like require and module.

Another package, @types/webpack-env exists to describe the intrinsic environment that programs have when they are bundled by webpack (often run in the browser). Webpack defines or overrides identifiers like require and module for the purpose of implementing its loader system.

For all intents and purposes, these two “environments” do not exist at the same time at runtime. Code that is bundled with webpack is going to use webpack’s require and module, and otherwise, code that runs in node is going to use node’s require and module. As a user, for any given file, you probably know which one of those environments is relevant to you, and you would like the types of your globals to reflect that.

However, it’s often the case that TypeScript projects end up with both environments’ global declarations at the same time. This is happening to @storybook/react because it contains server code that references node built-ins like path and fs in the same TypeScript project as client code, and they’ve explicitly included @types/webpack-env to cover that. The result is that, when generating the .d.ts file for a @storybook/react TypeScript file that references a global that’s defined in both places, the compiler spits out a reference to both of the typings packages where that global is defined: https://unpkg.com/browse/@storybook/[email protected]/dist/client/preview/index.d.ts

/// <reference types="node" />
/// <reference types="webpack-env" />

Now, any TypeScript user who imports @storybook/react will have both of these packages pulled into their own program, affecting the type of their own globals. Because @storybook/react lists @types/node as a devDependency and not a dependency, the consumer may or may not even have @types/node installed. If they don’t, they’ll get an error. If they do, @storybook/react lost the opportunity to have any say over what version of @types/node the user should have, perhaps because they never intended the user to have to have it at all?

image

Ok, so why did this blow up all of a sudden? Last week, I merged an update from a @types/node contributor on DefinitelyTyped adding typings for node 13. That update made minor changes to the shape of some of the same globals that @types/webpack-env declares.

When a global interface is declared two different times in the same TypeScript program, they get merged together, but if both interfaces declare the same property, the type of that property must be identical between the two declarations. When @types/node was updated, those interfaces were no longer mergeable with the ones in @types/webpack-env.

So, there are potentially two paths forward here:

  1. Make an update to @types/webpack-env to be compatible with @types/node@13
  2. Fix @storybook/react to ensure its any typings it has that are intended to be consumed on the client do not reference @types/node

In a perfect world, the former would not matter at all. Since code doesn’t use webpack’s loader system and node’s loader system at the same time, why would it matter if those two packages can coexist cleanly? But in the real world, it seems that an effort has been made in the past to keep webpack-env from clobbering node, since this problem is so common. So, we should probably pursue this course of action if possible.

But, we should investigate the latter as well. Users need some way of isolating the types for node and the types for webpack from each other, and right now @storybook/react (and @storybook/types) are making it impossible to create that boundary. This probably means splitting client and server into separate TypeScript projects throughout Storybook. (And if it was intentional to require users to have @types/node installed, then it needs to be listed as a dependency or peerDependency.) Unfortunately, I’m not a Storybook user, so I’m afraid I don’t know enough about how and where users import things from different Storybook packages to be able to do this myself. But I’m happy to help out with knowledge from the TS side.

@rkristelijn
Copy link

rkristelijn commented Jan 28, 2020

Same issue in @storybook/[email protected] and @storybook/[email protected]; setting skipLibCheck in.storybook/tsconfig.json is awork-around

{
  "...":"...",
  "compilerOptions": { "skipLibCheck": true },
  "...":"..."
}

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