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
Overload defineConfig to accept a RollupOptionsFunction parameter #4728
Conversation
I'm not really fond of the naming... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea
I'm not really fond of the naming...
neither am I. Why not just overload defineConfig
a third time to also accept a RollupConfigFunction
as parameter?
Codecov Report
@@ Coverage Diff @@
## master #4728 +/- ##
=======================================
Coverage 99.03% 99.03%
=======================================
Files 217 217
Lines 7690 7690
Branches 2126 2126
=======================================
Hits 7616 7616
Misses 24 24
Partials 50 50
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Why making it simple when we can make it complicated? 😃 You're absolutely right, I'm updating the PR ASAP. 🙏 |
Also renamed the type from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me, consider if you want to improve the test descriptions, will merge this in the next days.
test/load-config-file/index.js
Outdated
@@ -41,6 +41,15 @@ describe('loadConfigFile', () => { | |||
assert.deepStrictEqual(JSON.parse(JSON.stringify(options)), defaultConfigs); | |||
}); | |||
|
|||
it('loads an ESM config file exporting a config with defineConfigFn()', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit: The two test descriptions no longer fit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome 🎉
This PR has been released as part of rollup@3.5.1. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
This PR provides a
defineConfigFunction
auxiliary function, akin todefineConfig
but for configs that exports a function: