Dependency to @types/node makes electron package incompatible with typescript projects not using commonjs

[tothdavid]

The package installs @types/node package as dependency which is not compatible with e.g. @types/requirejs, as both defined require function in a different manner. This case is pretty common for browser typescript projects using AMD, with e.g. nightmarejs as headless browser for unit tests which actually uses electron.

• Electron version: 1.6.10
• Operating system: Windows

[MarshallOfSound]

@zeke Hm, this is one we didn't think of 🤔

I mean, technically the typings are accurate but the AMD definitions override the built in require function, maybe this is something that can be fixed in the AMD typings (somehow overriding the nodejs require declaration)

[tothdavid]

That's not actually the case: the Typescript compiler can't actually suppose you are using nodejs so there is actually no built in require function. That's why you need typings if you want to use the require function which is defined in e.g. @types/nodes or ˛@types/requirejs, but obviously they are not compatible so installing both packages will result in build error.

[MarshallOfSound]

@tothdavid The thing is Electron has all Node.JS API's (including require) built in, this is what I meant by the built in require function. If @types/requirejs needs to override that require I'm not sure how we can facilitate that or if it's something that needs to be fixed in those typings

[tothdavid]

Sure, that's perfectly fine that you have electron building on node.js, but why do you expose @types/node as a dependency when you don't even have any other .ts/d.ts in your package? Why is it not only a devDependency?

The thing I am telling you is that the @types/requirejs typings are not wrong, they are just different as commonjs is not really a valid option when your code runs in a browser. Also as I mentioned above using electron for running unit tests are a pretty common use case for any projects not just for the ones targeting node.js.

[MarshallOfSound]

why do you expose @types/node as a dependency when you don't even have any other .ts/d.ts in your package

We do? Check an install of electron, it will contain an electron typings file (only the more recent versions of electron have this file).

Also the latest version of this typings file is in the repo at the moment

https://github.com/electron-userland/electron-prebuilt/blob/master/electron.d.ts

The thing I am telling you is that the @types/requirejs typings are not wrong

I'm not saying they are, just saying there must be some way to resolve a conflict between two type declarations 😄

[tothdavid]

Oh, sorry, I checked the wrong version. :( But I can see it now.

You could still separate the typings from the main npm package and push the typings to a @types scoped package, but even typescript's recommendation is to publish them together: https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

Unfortunately require is a special function as it needs to be globally defined so there is not really a way (or at least none that I know of) to make both of them work in a single context.

[kenshyx]

Why not using this https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/electron/index.d.ts ?

[MarshallOfSound]

@kenshyx That type definition file is community maintained and published to the @types scope, we now publish our own first-party typings file generated at build time from our docs and ship it with the electron module.

[kenshyx]

Yeah, but it's a good base to use, right now that module will become unusable because they clash.

[MarshallOfSound]

@kenshyx First party typings are supposed to supersede the definitely typed modules. In the near future, the @types/electron package will probably be deprecated as people shouldn't be using both. They will just naturally get typings from the electron package.

[zeke]

The electron-prebuilt repo is being retired and its code has been moved into the electron/electron repo. For the sake of historical transparency, we will leave GitHub Issues enabled on this repository, but if you are still affected by the issue reported here, please open a new issue on electron/electron repo and reference this issue from it so people can get the full context. The electron repository has a large and active contributor community, so your issue is more likely to get the attention it deserves there. Thanks!

For this issue specifically, https://github.com/electron/electron-typescript-definitions might be the most appropriate place to continue the discussion.