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

refactor: change plugin export from default to named #274

Merged
merged 5 commits into from
Jul 31, 2023

Conversation

spiltbeans
Copy link
Contributor

@spiltbeans spiltbeans commented Jul 23, 2023

Describing the problem

If you import the plugin as directed in the docs you may encounter a console error like:

import dts from "rollup-plugin-dts";

const config = [
  // …
  {
    input: "./my-input/index.d.ts",
    output: [{ file: "dist/my-library.d.ts", format: "es" }],
    plugins: [dts()],
  },
];

export default config;
[!] TypeError: dts is not a function

And you will be returned this if you log dts to the console:

{ default: [Function: plugin] }

This PR closes #247.

Describing the solution

This PR modifies the export of the plugin function as a named export called dts rather than export default.

Using named exports is less likely to encounter these inconsistent importing patterns in the future.

This StackOverflow thread provides multiple reasons why named exports are a favourable pattern for ES6. This response compiles relevant articles expanding on the issue.

Describing changes to the package

This PR would change the way this package is imported into projects. It will be a breaking change as importing will now be:

// new import
import { dts } from "rollup-plugin-dts";

// example usage
const config = [
  // …
  {
    input: "./my-input/index.d.ts",
    output: [{ file: "dist/my-library.d.ts", format: "es" }],
    plugins: [dts()],
  },
];

export default config;

@spiltbeans spiltbeans marked this pull request as ready for review July 24, 2023 03:14
@spiltbeans
Copy link
Contributor Author

I've created an alternate PR in #275 to solve the same issue. They are different solutions and only one should be accepted.

Copy link
Owner

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of this. I would be curious about what part of the commonjs interop exactly is breaking this usecase here

src/index.ts Outdated Show resolved Hide resolved
tests/testcases.ts Outdated Show resolved Hide resolved
@Swatinem Swatinem merged commit 0e548c9 into Swatinem:master Jul 31, 2023
@Swatinem
Copy link
Owner

Thanks again for looking into this! I will get a new release out during this week.

@spiltbeans spiltbeans deleted the named-export branch September 22, 2023 14:15
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 this pull request may close these issues.

TypeError: dts is not a function
2 participants