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

TypeScript definitions for PluginContext are Incorrect #2974

Closed
wangweixuan opened this issue Jul 1, 2019 · 2 comments · Fixed by #2975
Closed

TypeScript definitions for PluginContext are Incorrect #2974

wangweixuan opened this issue Jul 1, 2019 · 2 comments · Fixed by #2975

Comments

@wangweixuan
Copy link

  • Rollup Version: 1.16.3
  • Operating System (or Browser): not concerned
  • Node Version: not concerned

How Do We Reproduce?

I want to write a plugin in TypeScript, for example:

import { PluginImpl } from 'rollup'

export const myPlugin: PluginImpl<{ }> = () => {
  return {
    name: 'my-plugin',

    transform(code, id) {
      this.error('There is something wrong.', 42) // Here
      return undefined
    }
  }
}

Expected Behavior

The code compiles and runs. The plugin reports an error at the 42nd character position in any file.

Actual Behavior

The code won't compile. TypeScript complains that Argument of type '42' is not assignable to parameter of type '{ column: number; line: number; }'.

What's Wrong

In Rollup's type definition file:

error: (err: RollupError | string, pos?: { column: number; line: number }) => never;

warn: (warning: RollupWarning | string, pos?: { column: number; line: number }) => void;

While in the document:
https://github.com/rollup/rollup/blob/master/docs/05-plugin-development.md#thiserrorerror-string--error-position-number--void

this.error(error: string | Error, position?: number) => void

https://github.com/rollup/rollup/blob/master/docs/05-plugin-development.md#thiswarnwarning-string--rollupwarning-position-number-

this.warn(warning: string | RollupWarning, position?: number)
@lukastaegert
Copy link
Member

Thanks for spotting this! As a matter of fact, both should work—at least there are tests for them. I will have a look at definitions and docs.

@lukastaegert
Copy link
Member

Fix at #2975

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