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

bug: ts issues in d.ts file #25

Closed
regevbr opened this issue Jun 14, 2020 · 7 comments · Fixed by #26
Closed

bug: ts issues in d.ts file #25

regevbr opened this issue Jun 14, 2020 · 7 comments · Fixed by #26

Comments

@regevbr
Copy link
Contributor

regevbr commented Jun 14, 2020

Hi @arcanis
My PR in snyk/nodejs-lockfile-parser#57 is using @yarnpkg/core which in turn depends on this library (2.4.1) but the d.ts file of the published npm files causes compilation issues.

node_modules/clipanion/lib/index.d.ts:2:23 - error TS2688: Cannot find type definition file for 'mocha'.

2 /// <reference types="mocha" />
                        ~~~~~

node_modules/clipanion/lib/index.d.ts:449:32 - error TS2339: Property 'binaryName$0' does not exist on type '{ binaryLabel?: string | undefined; binaryName?: string | undefined; binaryVersion?: string | undefined; enableColors?: boolean | undefined; }'.

449     constructor({ binaryLabel, binaryName$0, binaryVersion, enableColors }?: {
          

I will try to create a quick PR to fix it

@regevbr
Copy link
Contributor Author

regevbr commented Jun 14, 2020

The 2nd issue happens due to naming collision in the unified deceleration file and can be easily solved by giving the spread parameter a different name.

Spent a couple of hours investigating the 1st issue. I managed to understand that because @types/mocha declares a namespace for NodeJs -

declare namespace NodeJS {
    // Forward declaration for `NodeJS.EventEmitter` from node.d.ts.
    // Required by Mocha.Runnable, Mocha.Runner, and Mocha.Suite.
    // NOTE: Mocha *must not* have a direct dependency on @types/node.
    // tslint:disable-next-line no-empty-interface
    interface EventEmitter { }

    // Augments NodeJS's `global` object when node.d.ts is loaded
    // tslint:disable-next-line no-empty-interface
    interface Global extends Mocha.MochaGlobals { }
}

It causes typescript to assume that mocha is a referenced type. The strange thing is that if you run plain tsc command, the node referenced type is emitted but not the mocha one...

The issue first appears in the rollup-plugin-typescript in the typeReferenceCollector function (line 6465). I will keep investigating on how to resolve this.

@arcanis this can be easily solved if we separate the test dependencies, maybe by putting it in a different sub package.When I remove the @types/mocha as a dependency the issue got resolved...

p.s
I tried other rollup typescript plugins but hey all failed to run smoothly to begin with :-(

@regevbr regevbr changed the title ts issues in d.ts files bug: ts issues in d.ts files Jun 14, 2020
regevbr added a commit to PruvoNet/clipanion that referenced this issue Jun 14, 2020
regevbr added a commit to PruvoNet/clipanion that referenced this issue Jun 14, 2020
@regevbr
Copy link
Contributor Author

regevbr commented Jun 14, 2020

So it seems that solution was easy - declare explicitly the types in tsconfig and not put mocha there when bundling

@regevbr regevbr changed the title bug: ts issues in d.ts files bug: ts issues in d.ts file Jun 14, 2020
@arcanis
Copy link
Owner

arcanis commented Jun 15, 2020

The 2nd issue happens due to naming collision in the unified deceleration file and can be easily solved by giving the spread parameter a different name.

What is binaryName colliding with? What makes it different from, say, binaryLabel? 🤔 Is it just a bug in TS?

@regevbr
Copy link
Contributor Author

regevbr commented Jun 15, 2020

Seems to be a bug in the rollup ts plugin. I will open 2 issues there later today with reproduction repos

@regevbr
Copy link
Contributor Author

regevbr commented Jun 15, 2020

The roolup ts plugin causes the issue, it thinks it collides with binaryName from Core.ts for some reason. This is not a typescript issue but the plugins way to handle conflicts while merging the deceleration files.

arcanis added a commit that referenced this issue Jun 15, 2020
fix: ts issues in d.ts file #25
@arcanis
Copy link
Owner

arcanis commented Jun 15, 2020

Released in 2.4.2

@regevbr
Copy link
Contributor Author

regevbr commented Jun 15, 2020

Thanks @arcanis !

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 a pull request may close this issue.

2 participants