Skip to content

Commit ad1826e

Browse files
author
Kent C. Dodds
authored
feat(rollup): simplify external (#80)
<!-- Thanks for your interest in the project. Bugs filed and PRs submitted are appreciated! Please make sure that you are familiar with and follow the Code of Conduct for this project (found in the CODE_OF_CONDUCT.md file). Also, please make sure you're familiar with and follow the instructions in the contributing guidelines (found in the CONTRIBUTING.md file). If you're new to contributing to open source projects, you might find this free video course helpful: http://kcd.im/pull-request Please fill out the information below to expedite the review and (hopefully) merge of your pull request! --> <!-- What changes are being made? (What feature/bug is being fixed here?) --> **What**: basically this: Unless you're bundling a `umd` file, all files in `node_modules` should be marked as external. That's what this code does. <!-- Why are these changes necessary? --> **Why**: I noticed that when I upgraded to emotion 10 in one of my packages and started using the babel plugin it swapped `@emotion/styled` for `@emotion/styled-base` which is not marked as an external with this logic and it kinda messed things up. So I improved the external to be what I think is intended. <!-- How were these changes implemented? --> **How**: the predicate function is called with the same module multiple times which is what made this more tricky than I thought. First it's called with the module id as it appears in the code, then it's called with the full path to the module. <!-- Have you done all of these things? --> **Checklist**: <!-- add "N/A" to the end of each line that's irrelevant to your changes --> <!-- to check an item, place an "x" in the box like so: "- [x] Documentation" --> - [ ] Documentation N/A - [ ] Tests N/A - [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? --> - [ ] Added myself to contributors table N/A <!-- this is optional, see the contributing guidelines for instructions --> <!-- feel free to add additional comments --> I'd love some input from everyone who's touched the rollup config here. @Andarist, @alexandernanberg, @eliperkins, and @fobbyal. I think this is good, but it'd be nice to get a sanity check. Thanks!
1 parent 5d99117 commit ad1826e

File tree

1 file changed

+13
-2
lines changed

1 file changed

+13
-2
lines changed

src/config/rollup.config.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,19 @@ if (isPreact) {
8888
}
8989

9090
const externalPattern = new RegExp(`^(${external.join('|')})($|/)`)
91-
const externalPredicate =
92-
external.length === 0 ? () => false : id => externalPattern.test(id)
91+
92+
function externalPredicate(id) {
93+
const isDep = externalPattern.test(id)
94+
if (umd) {
95+
// for UMD, we want to bundle all non-peer deps
96+
return isDep
97+
}
98+
// for esm/cjs we want to make all node_modules external
99+
// TODO: support bundledDependencies if someone needs it ever...
100+
const isNodeModule = id.includes('node_modules')
101+
const isRelative = id.startsWith('.')
102+
return isDep || (!isRelative && !path.isAbsolute(id)) || isNodeModule
103+
}
93104

94105
const filename = [
95106
pkg.name,

0 commit comments

Comments
 (0)