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

Fixes Storybook/Webpack Out of Memory Error by using relative path to config stories location #1509

Merged
merged 2 commits into from
Jan 6, 2021

Conversation

dthyresson
Copy link
Contributor

Fixes #1496

An out of memory error occurs in my project:

info => Loading presets
info => Loading presets
info => Loading custom babel config as JS
info => Loading custom babel config as JS
info => Loading config/preview file in "../node_modules/@redwoodjs/core/config/storybook".
info => Adding stories defined in "../node_modules/@redwoodjs/core/config/storybook/main.js".
info => Using default Webpack setup.
info => Using base config because react-scripts is not installed.
webpack built 925c20b607c416ca0164 in 9318ms
╭───────────────────────────────────────────────────╮
│                                                   │
│   Storybook 5.3.21 started                        │
│   9.18 s for manager and 10 s for preview         │
│                                                   │
│    Local:            http://localhost:7910/       │
│    On your network:  http://192.168.7.53:7910/    │
│                                                   │
╰───────────────────────────────────────────────────╯
webpack building...
webpack built ff9c7c60946b6ae54aee in 15906ms
webpack building...
70% building 2/2 modules 0 active
<--- Last few GCs --->

[98773:0x108008000]   172516 ms: Mark-sweep (reduce) 4034.6 (4104.4) -> 4032.2 (4104.4) MB, 4576.8 / 5.2 ms  (average mu = 0.163, current mu = 0.061) allocation failure scavenge might not succeed
[98773:0x108008000]   177124 ms: Mark-sweep (reduce) 4033.2 (4101.4) -> 4032.4 (4102.9) MB, 4601.4 / 5.1 ms  (average mu = 0.087, current mu = 0.001) allocation failure scavenge might not succeed


<--- JS stacktrace --->

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory

I determined that the root cause -- and as with several other recent issues -- had to do with spaces and other problem characters in my path.

I believe that Storybook was trying to look at some other very "root"-like or user paths for stories which could contain a huge number of files and the splat simply caused an out of memory error while traversing it.

The following change from:

${getPaths().web.src}/**/*.stories.{tsx,jsx,js}.replace(/\\/g, '/'),

to a relative path

'../../../../web/src/**/*.stories.{tsx,jsx,js}',

as in:

module.exports = {
  stories: [
    // `${getPaths().web.src}/**/*.stories.{tsx,jsx,js}`.replace(/\\/g, '/'),
    '../../../../web/src/**/*.stories.{tsx,jsx,js}',
  ],

Worked.

This PR now uses a relative path to configure where Storybook should find the stories.

Questions for @peterp and others like @Tobbe (who is on Windows):

  • Is ok to use relative path?
  • If not and still want to do an absolute path based on getPaths().web.src the replace here is not doing the same thing as the ensurePosixPath from internal paths. Should it?

@github-actions
Copy link

github-actions bot commented Nov 20, 2020

@@ -6,7 +6,7 @@ const { getSharedPlugins } = require('../webpack.common')

module.exports = {
stories: [
`${getPaths().web.src}/**/*.stories.{tsx,jsx,js}`.replace(/\\/g, '/'),
'../../../../web/src/**/*.stories.{tsx,jsx,js}',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no way to solve this via improvement to getPaths()?

I'm not sure if this is correct, but I feel like we keep running into various path issues, patching them, and then running into them once again down the road.

Seems soon we need improved util functions that handle all cases and have corresponding tests. @dthyresson Maybe step one is to get this in. Then in a separate PR come back to getPaths() and harden appropriately.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thedavidprice > Is there no way to solve this via improvement to getPaths()?

Perhaps. But from what I have seen, paths are often used in different contexts and depending on that context how they are "formed" or escaped (or not) matters.

I initially thought the same -- fix the origin "path" and all downhill will work fine.

But some cases still failed.

One such case is the DATABASE_URL used as an env-ar in the Prisma schema. If that has extra " or was escaped, it failed.

Are you suggesting that in the internal paths here:

    web: {
      routes,
      base: path.join(BASE_DIR, 'web'),
      pages: path.join(BASE_DIR, PATH_WEB_DIR_PAGES),
      components: path.join(BASE_DIR, PATH_WEB_DIR_COMPONENTS),
      layouts: path.join(BASE_DIR, PATH_WEB_DIR_LAYOUTS),
      src: path.join(BASE_DIR, PATH_WEB_DIR_SRC),
...

Instead of path.join can a function that escape-path-with-spaces and does a path.join()?

Not sure that will help with processPagesDir since it also does its own path.joins or ensurePosixPath that does its own.

Should ensurePosixPath really be escape-path-with-spaces?

For this PR I will try to use escape-path-with-spaces on the Storybook stories config using the existing get paths and see for that fairs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried

module.exports = {
  stories: [
    `${escape(getPaths().web.src)}/**/*.stories.{tsx,jsx,js}`
  ],

and unfortunately, this did not help.

I still got memory errors because of the path:

╰───────────────────────────────────────────────────╯
webpack building...
10% building 2/2 modules 0 active
<--- Last few GCs --->

[97886:0x110008000]   241721 ms: Scavenge (reduce) 4027.7 (4100.7) -> 4027.1 (4101.7) MB, 4.2 / 0.0 ms  (average mu = 0.089, current mu = 0.032) allocation failure 
[97886:0x110008000]   241731 ms: Scavenge (reduce) 4027.7 (4100.7) -> 4027.2 (4102.0) MB, 5.7 / 0.0 ms  (average mu = 0.089, current mu = 0.032) allocation failure 


<--- JS stacktrace --->

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
 1: 0x1012b8fd5 node::Abort() (.cold.1) [/Users/dthyresson/.nvm/versions/node/v14.7.0/bin/node]

Perhaps because not only do I have a space, but I also have parentheses.

So far the only approach that works is to:

  • enclose these paths in double quotes as needed
  • relative paths

@Tobbe
Copy link
Member

Tobbe commented Nov 20, 2020

I'd be happy to give this a run through, but I don't currently have a good project to run it on. @dthyresson Do you have project up on GitHub I could clone and run this on?

I tried running storybook on just the tutorial project (only first part of the tutorial completed), but didn't get any Out of Memory errors. I get some other errors, but I don't think they're related, and storybooks seems to start and run fine

image

@dthyresson
Copy link
Contributor Author

I don't currently have a good project to run it on.

What I do is:

  • create a directory path that would be problematic somewhere (like ~/projects/My Bad (Path)/redwood/)
  • create a new redwood app as usual in there
  • run stock generators for a layout, page and cell and keep the default generated files (since these make stories)
  • run storybook

@Tobbe
Copy link
Member

Tobbe commented Nov 21, 2020

What I do is:

That's basically what I did too, except I only have spaces in my path, not parentheses. So will try that as well

@Tobbe
Copy link
Member

Tobbe commented Nov 21, 2020

So, with parentheses I still don't get any OOM errors. But it also doesn't work 😜

image

image

As you can see in the screen shot above, it did some funny quoting with the path, starting at the left parenthesis. I will try with your fix now.

@Tobbe
Copy link
Member

Tobbe commented Nov 21, 2020

Updated to RW 0.21 and for some reason the quoting is different

image

Still doesn't find any stories though.

@dthyresson Here's the output after applying your fix

image

And with that fix, StoryBook works

image

@Tobbe
Copy link
Member

Tobbe commented Nov 21, 2020

TL;DR I don't seem to get Out of Memory errors, but I do get other errors. @dthyresson's changes fixes my errors, and StoryBook works.

@dthyresson
Copy link
Contributor Author

@Tobbe confirms that the relative path approach works on Windows as well -- with a path that has spaces and parentheses.

@thedavidprice thedavidprice merged commit 3e9c379 into redwoodjs:main Jan 6, 2021
@thedavidprice thedavidprice added this to the Next Release milestone Jan 6, 2021
dac09 added a commit to dac09/redwood that referenced this pull request Jan 12, 2021
…h-tokens

* 'main' of github.com:redwoodjs/redwood:
  Move whatwg-fetch from devDep to dep
  Adds mockCurrentUser() to api-side jest
  v0.22.1
  v0.22.0
  Revert "Configure ApolloProvider to always use fresh tokens  (redwoodjs#1609)" (redwoodjs#1611)
  Configure ApolloProvider to always use fresh tokens  (redwoodjs#1609)
  Ignore *.scenarios.* files. (redwoodjs#1607)
  upgrade prisma v2.12.1 (redwoodjs#1604)
  Test Scenarios (redwoodjs#1465)
  Use relative path to config stories location (redwoodjs#1509)
@dthyresson dthyresson deleted the dt-fix-storybook-path-1496 branch December 23, 2021 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Out of memory error when invoking Storybook
4 participants