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

test(node-resolve): add tests for mixing custom exportConditions with browser: true #1043

Merged

Conversation

Andarist
Copy link
Member

@Andarist Andarist commented Nov 16, 2021

Rollup Plugin Name: @rollup/plugin-node-resolve

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
  • no

Description

I was wondering if this would work correctly - I've figured out that the quickest way to found out would be to check the test suite but I couldn't find any exportConditions-related test. The only place where "exportCondition:" string appears in the codebase is this type test:

exportConditions: ['a', 'b'],

Since it felt like such tests are missing, I've decided to just add them 😉

Copy link
Contributor

@guybedford guybedford 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 posting these. Note the lint issue / console.log.

@Andarist Andarist force-pushed the node-resolve-browser-field-mixed-custom-condition-test branch from 40c5f41 to 28b14c7 Compare November 16, 2021 23:23
@Andarist
Copy link
Member Author

@guybedford fixed 🤦

@Andarist
Copy link
Member Author

Andarist commented Nov 18, 2021

Since this is approved and CI passes - can I just land this? I'm slightly unsure and I don't want to get in the way of The Process 😉

@shellscape shellscape changed the title chore(test): add tests for mixing custom exportConditions with browser: true test(node-resolve): add tests for mixing custom exportConditions with browser: true Dec 13, 2021
@shellscape shellscape merged commit 9e6db2d into master Dec 13, 2021
@shellscape shellscape deleted the node-resolve-browser-field-mixed-custom-condition-test branch December 13, 2021 13:59
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