-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Allow to pass "true" to InputOptions.cache #4859
Conversation
Vite has changed the default of this option to "false" (vitejs/vite#11454), therefore it is needed to be able to set it to "true" explicitly. This may be useful in other situations too, e.g. extending some defaults options object which contains this option set to "false".
✅ Deploy Preview for rollupjs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
Yes you are right, that change makes sense. Some additional points, though:
- Currently, TypeScript tests are failing and Rollup does not build
- The documentation should be adapted in
docs/configuration-options/index.md
to change the type for thecache
option and note thattrue
is the default.
@lukastaegert Are the TS tests failing due to my change, or in general? (I have created the PR with the Gitlab inline editor). |
Definitely due to your changes. We do not merge PRs if those tests fail, and CI makes sure this is not overlooked. Actually, it was the Netlify deploy that already failed, but you can inspect the test results yourself. For that reason, I cannot recommend to do any production code changes in the inline editor. If you do not want to download it locally, try StackBlitz https://pr.new/github.com/danielrentz/rollup/tree/patch-1 |
I see, thanks for explaining. Will take care. |
I decided to leave "NormalizedInputOptions" as-is (type |
Codecov Report
@@ Coverage Diff @@
## master #4859 +/- ##
==========================================
- Coverage 98.98% 98.97% -0.02%
==========================================
Files 219 219
Lines 7898 7899 +1
Branches 2189 2190 +1
==========================================
Hits 7818 7818
Misses 26 26
- Partials 54 55 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Seems at this point only the markdown linter is complaining, but I can fix that for you. |
This PR has been released as part of rollup@3.16.0. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
Vite has changed the default of this option to "false" (vitejs/vite#11454), therefore it is needed to be able to set it to "true" explicitly. This may be useful in other situations too, e.g. extending some defaults options object which contains this option set to "false".