search

orbitjs / orbit

Composable data framework for ambitious web applications.
https://orbitjs.com
MIT License
2.34k stars 133 forks source link

Add "sideEffects": false to Orbit packages #969

Closed DanielGiljam closed 2 years ago

DanielGiljam commented 2 years ago

Adding "sideEffects": false to each Orbit packages’ package.json would allow build tools to tree shake the packages when bundling code for e.g. a web app.

dgeb commented 2 years ago

@DanielGiljam thanks for the suggestion! I believe all of Orbit's packages are indeed side-effect-free. Would you like to PR this change?

DanielGiljam commented 2 years ago

@dgeb Yes, gladly! I agree that Orbit's packages are side-effect-free in the sense that we can add "sideEffects": false, even though there's technically at least one place (potentially several places) where I can think of side-effects occurring.

In packages/@orbit/core/src/main.ts, the OrbitGlobal object is created and exported. If I'm not mistaken, this is technically a side-effect. If instead, a function was exported, and when you call the function, then the object would be created, then it would be side-effect-free.

I don't think side-effects of this kind make the library any less tree-shakeable, so I don't think it should prevent us from adding "sideEffects": false.

However, making the library 100% side-effect-free could be done fairly easily. One would just have to go through the code and change all the places where it can't be statically analyzed as side-effect-free. But it would require a larger PR and it may not serve any real purpose. So perhaps adding "sideEffects": false is enough?

dgeb commented 2 years ago

@DanielGiljam after a bit of review, I agree that it's probably fine to mark all the public packages as free of side-effects. The OrbitGlobal export was my concern as well, but I think it should be fine because it is not modified and then re-exported by orbit's own packages.

If instead, a function was exported, and when you call the function, then the object would be created, then it would be side-effect-free.

I don't think we could take this approach because modifications to the Orbit global singleton are the means by which an app (or lib that depends on Orbit) can override functions such as uuid or the debug setting. I'm assuming that the function export you're proposing would create a new object on each call?

I'm going to merge #972 and will get a new release out shortly. Please let me know if you see any unexpected behavior with tree-shaking. Thanks again for your work on this.