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

feat(node-resolve): Add default export #361

Merged
merged 1 commit into from
May 14, 2020

Conversation

NotWoods
Copy link
Member

@NotWoods NotWoods commented May 1, 2020

Rollup Plugin Name: node-resolve

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no (this is a pretty straightforward change)

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary) Since we're including breaking changes anyways, I'm going to fix the Typescript interface name so it matches other plugins.
  • no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

Building on #301, this updates the documentation but also re-adds a default export. When the plugin is used with ES modules, the previous change is no longer a breaking change. However, CommonJS modules still must switch to named exports.

For consistency we should always used named exports in the documentation, but this helps users migrate to the new format.

@NotWoods NotWoods force-pushed the feat/node-resolve/default-export branch from 81be79b to ce8fed8 Compare May 1, 2020 22:16
@TrySound
Copy link
Member

TrySound commented May 1, 2020

Do you mean "Add named export"?

@NotWoods
Copy link
Member Author

NotWoods commented May 2, 2020

We've switched to named exports already here, this adds back an export named "default".

@@ -140,9 +140,9 @@ Example:
```javascript
// rollup.config.js
import alias from '@rollup/plugin-alias';
import resolve from '@rollup/plugin-node-resolve';
import { nodeResolve } from '@rollup/plugin-node-resolve';
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's keep this scoped to node-resolve for now, and update the plugins individually. we'll want to do a patch release for each so the docs on npm match the repo. for that we need individual commits for each plugin to keep the changelogs and publishing automation correct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@NotWoods ping. can we do one PR per plugin or a separate commit per plugin? With the path forward in #360 I'm thinking that a separate PR might be easier to digest.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll revert these doc changes since the path forward should allow us to keep the same import format.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome

@NotWoods NotWoods force-pushed the feat/node-resolve/default-export branch from ce8fed8 to e99441a Compare May 11, 2020 15:25
@shellscape shellscape merged commit fe037db into rollup:master May 14, 2020
larrybotha added a commit to larrybotha/plugins that referenced this pull request May 20, 2020
# By Tiger Oakes (2) and others
# Via GitHub
* master:
  feat(wasm): Switch to TypeScript & named exports (rollup#363)
  feat(node-resolve): Add default export (rollup#361)
  fix (sucrase): resolve directory imports (rollup#390)
  docs(typescript): update readme examples (rollup#391)

# Conflicts:
#	packages/sucrase/test/snapshots/test.js.md
#	packages/sucrase/test/snapshots/test.js.snap
#	packages/sucrase/test/test.js
#	pnpm-lock.yaml
LarsDenBakker pushed a commit to LarsDenBakker/plugins that referenced this pull request Sep 12, 2020
BREAKING CHANGES:

- Corrects TypeScript interface name
- Adds default export
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants