Skip to content
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

Add "sideEffects": false to Orbit packages #969

Closed
DanielGiljam opened this issue Aug 16, 2022 · 3 comments
Closed

Add "sideEffects": false to Orbit packages #969

DanielGiljam opened this issue Aug 16, 2022 · 3 comments

Comments

@DanielGiljam
Copy link
Contributor

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
Copy link
Member

dgeb commented Aug 31, 2022

@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
Copy link
Contributor Author

@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
Copy link
Member

dgeb commented Sep 22, 2022

@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.

@dgeb dgeb closed this as completed Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants