Skip to content

react-spring SideEffects must be set to true #30454

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

Closed
joshuaellis opened this issue Mar 25, 2021 · 4 comments
Closed

react-spring SideEffects must be set to true #30454

joshuaellis opened this issue Mar 25, 2021 · 4 comments
Labels
topic: webpack/babel Webpack or babel type: question or discussion Issue discussing or asking a question about Gatsby

Comments

@joshuaellis
Copy link

Description

This issue has been reported to us a few times see: pmndrs/react-spring#1239 & pmndrs/react-spring#1069 as an example. Peoples workaround is to enforce sideEffects:true on the particular package:

exports.onCreateWebpackConfig = ({ stage, actions }) => {
  if (stage.startsWith("build-javascript")) {
    actions.setWebpackConfig({
      module: {
        rules: [
          {
            test: /react-spring/,
            sideEffects: true
          }
        ]
      }
    })
  }
}

It'd be great to get your feedback on whether this is the intended fix for the issue or it's a bug in either library that can be fixed. If it's the correct thing to do then I can add some documentation to let everyone know so neither library gets the bugs.

@joshuaellis joshuaellis added the type: bug An issue or pull request relating to a bug in Gatsby label Mar 25, 2021
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Mar 25, 2021
@herecydev
Copy link
Contributor

As far as I can see, Gatsby is doing nothing special in this regard and is delegating this decision making to webpack.There's some documentation here https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free that highlights the difficulties.

it's necessary to provide hints to webpack's compiler on the "pureness" of your code.

Are there any side effects in react spring that you are aware of? Perhaps it's prudent to explicitly call out those files so webpack can function better.

Although the workaround works, it's surely much better that react spring defines this in its package.json and then all webpack users will benefit from this change.

@joshuaellis
Copy link
Author

I think the library has specifically removed sideEffects:false from it's package files according to this issue – pmndrs/react-spring#1078 I've asked the original author of the issue in spring to try the newest package and see if that works.

It doesn't sound like we have to explicitly declare sideEffects:true instead?

Although the workaround works, it's surely much better that react spring defines this in its package.json and then all webpack users will benefit from this change.

I definitely agree, I'm just trying to get my head round what sideEffects actually were, so thank you for the article 👍🏼 It explains why this issue is so important pmndrs/react-spring#1158

@herecydev
Copy link
Contributor

Yes I agree, false would indicate there's no side effects and that could further inflict issues.

True would almost definitely solve the issue although its heavy handed. Specifically highlighting the sideffect files seems to be the utopia but might be complex, as there are a number of calls to Globals.assign in several files for your packages.

@LekoArts LekoArts added topic: webpack/babel Webpack or babel type: question or discussion Issue discussing or asking a question about Gatsby and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer type: bug An issue or pull request relating to a bug in Gatsby labels Mar 26, 2021
@joshuaellis
Copy link
Author

Specifically highlighting the sideffect files seems to be the utopia but might be complex, as there are a number of calls to Globals.assign in several files for your packages.

I agree, it's definitely something to look into. Thanks for the insight 👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: webpack/babel Webpack or babel type: question or discussion Issue discussing or asking a question about Gatsby
Projects
None yet
Development

No branches or pull requests

3 participants