Skip to content

EEGVisEnabled check issue and webpack cache #9159

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.

Sign up for GitHub

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

Jump to bottom
Merged
driusan merged 2 commits into from
Mar 27, 2024
Merged

EEGVisEnabled check issue and webpack cache #9159

driusan merged 2 commits into from
Mar 27, 2024

Conversation

@laemtl
Copy link
Contributor

@laemtl laemtl commented Mar 21, 2024 •
edited
Loading

  • Fix a WebpackMissingModule error with the EEGBrowser when EEGVisEnabled is set to "1" | "0" instead of "true" | "false".
  • Turn the webpack cache to off as it contributes to the heap memory issues for users with less than 2GB Ram. In dev mode, all cache is stored in memory. Alternatively, if build speed is impacted, it can also be set to filesystem.

@laemtl laemtl requested a review from kongtiaowang March 21, 2024 22:59
kongtiaowang
kongtiaowang approved these changes Mar 22, 2024
View reviewed changes
Copy link
Contributor

@kongtiaowang kongtiaowang left a comment

Choose a reason for hiding this comment

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

LGTM

@kongtiaowang kongtiaowang added the 26.0.0-bugs Issues that were raised during the release testing for 26.0.0 label Mar 22, 2024
@driusan
Copy link
Collaborator

driusan commented Mar 22, 2024

What does "and webpack cache" mean? Why is the cache: false flag being set?

@laemtl
Copy link
Contributor Author

laemtl commented Mar 24, 2024

Sorry, I forgot to explain that change in the description. Updated.

@driusan
Copy link
Collaborator

driusan commented Mar 25, 2024

@GeorgeMurad can you test if this branch fixes your memory issue problems when running npm run compile?

@laemtl is there any reason not to change it to cache to the filesystem instead of disabling it entirely?

@laemtl
Copy link
Contributor Author

laemtl commented Mar 25, 2024 •
edited
Loading

@laemtl is there any reason not to change it to cache to the filesystem instead of disabling it entirely?

No, it might be a good option as well, maybe a better compromise.

@christinerogers christinerogers mentioned this pull request Mar 25, 2024
Closed
3 tasks
@christinerogers christinerogers added the Critical to release PR or issue is key for the release to which it has been assigned label Mar 26, 2024
@christinerogers
Copy link
Contributor

christinerogers commented Mar 26, 2024

resolution of this issue is critical to release for the EEG Browser (signal display breaks if not #9160 )

@ridz1208
Copy link
Collaborator

ridz1208 commented Mar 26, 2024

@laemtl could you make the change for filesystem cache

@laemtl laemtl force-pushed the EEGVisEnabled-check-issue-and-webpack-cache branch 2 times, most recently from 6f95b62 to 0049008 Compare March 26, 2024 19:19
@laemtl
Copy link
Contributor Author

laemtl commented Mar 26, 2024

@ridz1208 Done

@driusan
Copy link
Collaborator

driusan commented Mar 26, 2024

@laemtl the static tests are failing

@laemtl laemtl force-pushed the EEGVisEnabled-check-issue-and-webpack-cache branch from 0049008 to 94974f7 Compare March 26, 2024 20:42
@driusan driusan merged commit b7ff2c6 into aces:main Mar 27, 2024
@ridz1208 ridz1208 added this to the 26.0.0 milestone Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kongtiaowang kongtiaowang kongtiaowang approved these changes

Assignees

@laemtl laemtl

Labels

26.0.0-bugs Issues that were raised during the release testing for 26.0.0 Critical to release PR or issue is key for the release to which it has been assigned

Projects

None yet

Milestone

26.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@laemtl @driusan @christinerogers @ridz1208 @kongtiaowang