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

Revert the extension path to not expand to sub-directories to stop OOM #417

Closed
wants to merge 1 commit into from

Conversation

robdimarco
Copy link

After upgrading to 2.0, I was consistently running out of RAM when trying to run webpack.

After some digging, I came across a comment on the webpack project that helped shed some light.

webpack/webpack#1914 (comment)

It seems that in PR 201 (#201), the extensionGlob
was changed from *{${paths.extensions.join(',')}}* to **/*{${paths.extensions.join(',')}}*

This change removes the **/ in the extension and fixes my OOM issue.

I am not sure about the ramifications for allowing pack files in subdirectories
(the original intent of the PR that introduced the change). Maybe we need a comment or a
flag when generating the configuration that will set it one way or the other?

…M error.

After upgrading to 2.0, I was consistently running out of RAM when trying to run webpack.

After some digging, I came across a comment on the webpack project that helped shed some light.

webpack/webpack#1914 (comment)

It seems that in PR 201 (rails#201), the `extensionGlob`
was changed from `*{${paths.extensions.join(',')}}*` to `**/*{${paths.extensions.join(',')}}*`

This change removes the `**/` in the extension and fixes my OOM issue.

I am not sure about the ramifications for allowing pack files in subdirectories
(the original intent of the PR that introduced the change). Maybe we need a comment or a
flag when generating the configuration that will set it one way or the other?
@gauravtiwari
Copy link
Member

@robdimarco What files you have under packs or whatever folder you are using for entries/packs?

@robdimarco
Copy link
Author

robdimarco commented May 25, 2017

I have 5 files directly in app/javascript/packs and 131 files all told if you include files in subdirectories. This includes Jest tests (in __tests__ subdirectories).

@guilleiguaran
Copy link
Member

@joerodrig this looks similar to the issue you found the last week.

@guilleiguaran
Copy link
Member

I guess the official reply for this is that this is totally expected since we expect to have in app/javascripts/packs only the 'packs' that are equivalent to webpack entry points, anything else should be in app/javascripts but outside of packs, that's just the convention that we have adopted for this.

@robdimarco
Copy link
Author

OK, that works for me. Thanks.

@gauravtiwari
Copy link
Member

Perhaps we make this more clear in README 👍

@marcelkooi
Copy link

@robdimarco I'm curious to know if you've run into any issues with this change? We're looking to make this change for our app as well and have noticed that it reduces our deployment time substantially, but would like to make sure it doesn't have any downstream effects

@robdimarco
Copy link
Author

@Marz0 worked great for us. No side-effects.

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

Successfully merging this pull request may close these issues.

4 participants