-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
build: Do not exclude devDependencies from rollup build
#6358
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
Conversation
Currently, our rollup config adds all `dependencies`, `devDependencies` and `peerDependencies` to the `external` array, meaning they will not be inlined into the build. However, that makes it impossible to _selectively_ inline files. For example, in replay we'd like to inline `rrweb` into the bundle. What makes this even more tricky is that because of the nature of `deepmerge`, which we use to merge custom config into the default one, it is actually impossible to overwrite this, as a custom `external` array will not overwrite the default one, but be added to it. This change removes `devDependencies` from the externals. IMHO that is otherwise bound to break anyhow - as if the dependency is neither `dependency` nor `peerDependency`, and you import it (but not bundle it), stuff might break anyhow.
size-limit report 📦
|
Lms24
left a comment
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.
This change removes
devDependenciesfrom the externals. IMHO that is otherwise bound to break anyhow - as if the dependency is neitherdependencynorpeerDependency, and you import it (but not bundle it), stuff might break anyhow. If there is a dependency that you are referencing in a bundle, but it is not added asdependencies, you have to add it aspeerDependencies.
That's a good point and I generally agree with it. As we had broken releases in the past because of build changes, I'd be curious to get a second opinion here from @lobsterkatie as you're the expert on build-related things.
I'm generally curious if this combination of inlining rrweb and keeping our transpiled JS in separate files works. In case it doesn't we can still try bundling our JS into one file, like Replay did originally.
|
Yeah, agreed! Regarding how that works with rrweb, I tried it, and basically it puts a |
Just to confirm, into |
|
Awesome! |

Currently, our rollup config adds all
dependencies,devDependenciesandpeerDependenciesto theexternalarray, meaning they will not be inlined into the build.However, that makes it impossible to selectively inline files. For example, in replay we'd like to inline
rrwebinto the bundle.What makes this even more tricky is that because of the nature of
deepmerge, which we use to merge custom config into the default one, it is actually impossible to overwrite this, as a customexternalarray will not overwrite the default one, but be added to it.This change removes
devDependenciesfrom the externals. IMHO that is otherwise bound to break anyhow - as if the dependency is neitherdependencynorpeerDependency, and you import it (but not bundle it), stuff might break anyhow. If there is a dependency that you are referencing in a bundle, but it is not added asdependencies, you have to add it aspeerDependencies.I tried to locally
yarn clean && yarn buildand checked all thebuild/esmoutputs, and there was no unexpectednode_modulesstuff to be found there. We generally use few dependencies, so as far as I can tell this has no actual impact on current output.