Skip to content

RHSTOR-7462: Upgrade style-loader to 4.0.0 #2138

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

Conversation

NIKHITHAVADDEMPUDI
Copy link
Collaborator

https://issues.redhat.com/browse/RHSTOR-7462

Upgrade style-loader to 4.0.0

Upradation of style-loader to 4.0.0 is leading to the error:
Module build failed (from ./node_modules/thread-loader/dist/cjs.js):
Thread Loader (Worker 1)
loaderContext.utils.contextify is not a function

so thread loader is removed from package.json and webpack.config.ts
and upgraded style-loader to 4.0.0
style_loader_errors.txt

@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 1, 2025

@NIKHITHAVADDEMPUDI: This pull request references RHSTOR-7462 which is a valid jira issue.

In response to this:

https://issues.redhat.com/browse/RHSTOR-7462

Upgrade style-loader to 4.0.0

Upradation of style-loader to 4.0.0 is leading to the error:
Module build failed (from ./node_modules/thread-loader/dist/cjs.js):
Thread Loader (Worker 1)
loaderContext.utils.contextify is not a function

so thread loader is removed from package.json and webpack.config.ts
and upgraded style-loader to 4.0.0
style_loader_errors.txt

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid jira ticket of any type label Jul 1, 2025
Copy link
Contributor

openshift-ci bot commented Jul 1, 2025

@NIKHITHAVADDEMPUDI: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/odf-console-e2e-aws 718b4ff link true /test odf-console-e2e-aws

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

"swr": "2.3.3",
"thread-loader": "^4.0.4",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove the thread-loader: it'll cause build issues in our CI environment.
If any, style-loader should be removed as it's only for development purposes.

@SanjalKatiyar @bipuladh what about replacing style-loader with mini-css-extract-plugin?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't remove the thread-loader: it'll cause build issues in our CI environment.

AFAIU removing it shouldn't cause issues in CI env. Earlier "thread-loader" was being used with empty "options" (options: {}), in which case it was over utilising the resources. Removing it entirely might cause a slightly slower build time, but no errors.

If any, style-loader should be removed as it's only for development purposes.

I was not aware that: This loader is primarily meant for development. The default settings are not safe for production environments. So, shifting to mini-css-extract-plugin seems like a good idea !!

Copy link
Collaborator

@alfonsomthd alfonsomthd Jul 2, 2025

Choose a reason for hiding this comment

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

@SanjalKatiyar The improvements I made on that PR were to prevent build errors/timeouts so removing it will imply either consuming more resources (thus having errors like 137) or facing more timeouts.

Copy link
Collaborator

@SanjalKatiyar SanjalKatiyar Jul 2, 2025

Choose a reason for hiding this comment

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

I remember, but wasn't that because "thread-loader" was consuming as much resources as it can get for parallel processing (that's why we were limiting worker pool count in the CI env).

If we don't use "thread-loader" at all, we won't be in that situation anymore. However, theoretically build time would increase.

Anyway, I can understand "style-loader" has other issues, so yeah we should remove it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right: my point is that removing the thread-loader will likely cause CI timeouts and slower builds (but keeping it while removing its config will cause resource consumption errors like 137) so let's keep it with the current config.

Copy link
Contributor

openshift-ci bot commented Jul 7, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: NIKHITHAVADDEMPUDI
Once this PR has been reviewed and has the lgtm label, please ask for approval from alfonsomthd. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@SanjalKatiyar
Copy link
Collaborator

Will be done as part of: https://issues.redhat.com/browse/RHSTOR-7589 task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira/valid-reference Indicates that this PR references a valid jira ticket of any type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants