Wrap immutable.d.ts type declarations in a module to avoid a global declaration #1869
Conversation
|
Hi. Thanks for looking at this. There is still the global declaration. This causes typescript to be fine with it: |
|
The current definition file having a global namespace export is intentional since Immutable is a UMD package, and can be used as a global Immutable variable. The alternative would be to host two separate definition files for the two different use cases, but this single file pattern is fairly common across definitely-typed. For example the React definitions use this same pattern. |
|
Alright then. I did not study the problem of module type definitions very much. I see you did a good job on the types and surely have your reasons. I suspected it was done this way for a reason. Well anyway... Question is, how big of a crowd is the Typescript + UMD crowd? Well, I suspect this is as much as I can contribute. Whatever you decide, I don't think further discussion (me pushing my way) would be productive. Thanks for considering my proposal. And thanks for taking care of the package! You, guys (or whatever, one never knows), rock! |
|
As mentioned in slack, maybe it is worth testing the same-name pattern instead of the "types" definitions in the package.json. Place separate definitions [name].d.ts besides the umd and es6 scripts and it should be automatically picked up. Maybe it only works when working with folders (e.g. /dist/es6/ and /dist/umd/) or when directly referencing it (e.g. @angular/material uses this pattern |
The problem:
When using the type-definitions, you provide right now, this is possible and does not get detected not even by running
tsc:Notice, there's no import statement. When the code is run, it errors out, unless ProvidePlugin is used in WebPack to add the necessary import.
This is due to the global nature of the namespace declaration.
The solution:
Take the namespace declaration out of the global.
With the type definitions proposed in the pull request this happens:
Despite the form of import being proposed when imported the prefered way, it works just fine.
Tests pass as far as they can on a windows machine. Your npm scripts for
build:prepareare not windows compatible as the shebang notation has no effect on windows. But maybe adding the shebang to the package.json directly, resulting in"build:prepare": "sh -e ./resources/prepare-dist.sh"might help. Most unix-based devices have alias for "sh" to the preferred shell. Be itkshon a mac orbashon linux machines.