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

Flatten function performance issue v4 #2424

Closed
ghost opened this issue Mar 4, 2019 · 11 comments
Closed

Flatten function performance issue v4 #2424

ghost opened this issue Mar 4, 2019 · 11 comments

Comments

@ghost
Copy link

@ghost ghost commented Mar 4, 2019

Having recently tried to upgrade our styled-components from v3 to v4 in a relatively large react app - we've ran into a strange performance issue that seems to be tied to the flatten function. When commenting out this part of flatten, the issue seems to resolve:

let shouldThrow = false;
try {
// eslint-disable-next-line new-cap
if (isElement(new chunk(executionContext))) {
shouldThrow = true;
}
} catch (e) {
/* */
}
if (shouldThrow) {
throw new StyledError(13, getComponentName(chunk));
}

Here are screenshots of performance snapshots run with chrome dev tools when performing the same action in our app on v3 vs. v4:

v3
screen shot 2019-03-04 at 11 06 48 am

v4
screen shot 2019-03-04 at 10 59 45 am

Strangely, we have not been able to reproduce this in a clean slate react app.

Environment

System:

  • OS: macOS High Sierra 10.13.6
  • CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
  • Memory: 2.29 GB / 16.00 GB
  • Shell: 5.3 - /bin/zsh

Binaries:

  • Node: 8.14.0 - /usr/local/bin/node
  • Yarn: 1.13.0 - /usr/local/bin/yarn
  • npm: 6.5.0 - /usr/local/bin/npm

npmPackages:

  • babel-plugin-styled-components: ^1.4.0 => 1.10.0
  • styled-components: ^4.1.3 => 4.1.3

Reproduction

Unable to reproduce with a basic react app created via create-react-app.

Expected Behavior

Performance is as good if not better than v3.

Actual Behavior

Performance is sluggish, significantly slower than v3.

@alansouzati
Copy link
Contributor

@alansouzati alansouzati commented Mar 4, 2019

I'm wanting to give a try and fix this. do you have a way to reproduce this outside your app? can you publish something that I can clone and try?

@probablyup
Copy link
Contributor

@probablyup probablyup commented Mar 4, 2019

You can trigger that code path by passing a non-styled component SFC into a style interpolation.

@alansouzati
Copy link
Contributor

@alansouzati alansouzati commented Mar 4, 2019

let my try to create something that reproduces it in codesandbox. I usually like to reproduce the problem first before trying to attack it.

@ghost
Copy link
Author

@ghost ghost commented Mar 4, 2019

@alansouzati Unfortunately I've not been able to reproduce this outside our app. I'll keep you updated if I can reproduce / make any progress.

@alansouzati
Copy link
Contributor

@alansouzati alansouzati commented Mar 4, 2019

@mariel9999999 are you using the "component selector" pattern?

const Icon = styled.svg`
  flex: none;
  transition: fill 0.25s;
  width: 48px;
  height: 48px;

  ${Link}:hover & {
    fill: rebeccapurple;
  }
`;

see the Link component inside the Icon interpolation?

@ghost
Copy link
Author

@ghost ghost commented Mar 4, 2019

@alansouzati
Copy link
Contributor

@alansouzati alansouzati commented Mar 4, 2019

just as a test, if you remove it, does the problem go away too? I'm trying to debug what is going on but I'm pretty new to their source code.

@ghost
Copy link
Author

@ghost ghost commented Mar 4, 2019

@alansouzati Nope. It is still laggy after removing all component selectors.

@aczekajski
Copy link

@aczekajski aczekajski commented Mar 5, 2019

I couldn't pin-point my problems down to flatten function, but we're experiencing the same issue - after upgrading relatively big project to v4, it performs about two times slower than with v3.

@probablyup
Copy link
Contributor

@probablyup probablyup commented Mar 5, 2019

@mariel9999999 can you try styled-components@4.1.4-alpha.6 (@alansouzati's fix) and let me know if it improves your graphs at all?

@ghost
Copy link
Author

@ghost ghost commented Mar 5, 2019

@probablyup Yup that fixes it.

Here's what it looks like when performing the same action (left = styled-components@4.1.4-alpha.6 and right = 4.1.3):
screen shot 2019-03-05 at 1 29 34 pm

@alansouzati Thanks for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants