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

fix: exported types #23

Closed
wants to merge 1 commit into from
Closed

fix: exported types #23

wants to merge 1 commit into from

Conversation

@pd4d10
Copy link

@pd4d10 pd4d10 commented Sep 1, 2020

No description provided.

resolvers: [resolver],
configureServer: reactRefreshServerPlugin,
transforms: [reactRefreshTransform]
}

export = plugin

This comment has been hidden.

@antfu

antfu Sep 2, 2020

Suggested change
export = plugin
export default plugin

This comment has been minimized.

@pd4d10

pd4d10 Sep 2, 2020
Author

It would change the CommonJS API to require('vite-plugin-react').default. Is it in line with expectations?

This comment has been minimized.

@antfu

antfu Sep 3, 2020

You are right. Dismissed

This comment has been minimized.

@pd4d10

pd4d10 Sep 3, 2020
Author

BTW, I also think export default is better if breaking changes are accepted. (bump a major version)

This comment has been minimized.

@antfu

antfu Sep 3, 2020

Yep. Actually, if we use rollup for bundling, export default could be automatically converted to module.exports for cjs. However, as this is just a simple cjs plugin, there is no need to make things complicated for now.

@antfu
antfu approved these changes Sep 3, 2020
Copy link
Contributor

@aleclarson aleclarson left a comment

LGTM

@aleclarson
Copy link
Contributor

@aleclarson aleclarson commented Oct 4, 2020

Closes #7

@yyx990803
Copy link
Contributor

@yyx990803 yyx990803 commented Dec 3, 2020

Thanks for the PR - it seems I can't resolve the conflict since the PR doesn't allow me to push to it, so I did it in e2ba377. Hope you don't mind.

@yyx990803 yyx990803 closed this Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.