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(dsv): fix type of processRow option #1581

Merged
merged 3 commits into from
Sep 25, 2023
Merged

fix(dsv): fix type of processRow option #1581

merged 3 commits into from
Sep 25, 2023

Conversation

chika3742
Copy link
Contributor

@chika3742 chika3742 commented Sep 13, 2023

Rollup Plugin Name: dsv

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • 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: #1492

Description

(successor to #1493)

This fixes the type definition for the processRow option to the DSV plugin, so that:

  • a function that doesn't return anything can be used
  • a function that returns a row object that includes non-string values (such as number or Date) can be used

This PR also removes undefined from the return value type, since there is no case where undefined is explicitly returned.

@shellscape
Copy link
Collaborator

@chika3742 please update your branch from upstream so we can run the latest CI

… files (#1571)

* test(typescript): add test case for incorrect `sourceContent`

* fix(typescript): fix .js.map files being treated as declarations
@shellscape
Copy link
Collaborator

Looks like there are some CI failures that need a look

@shellscape shellscape merged commit 09bab88 into rollup:master Sep 25, 2023
5 checks passed
@chika3742 chika3742 deleted the fix/dsv-processrow-typing branch September 25, 2023 16:07
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.

3 participants