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 #37113 - Drop unused downshift dependency #10867

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Jan 26, 2024

What are the changes introduced in this pull request?

Removes an unused nodejs dependency.

Considerations taken when implementing this change?

Negative lines of code are better than positive!

What are the testing steps for this pull request?

Github Actions shall tell the tale.

@ekohl
Copy link
Member

ekohl commented Jan 27, 2024

Looks like it's unused since a1402c0

Copy link
Contributor

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

Failure appears unrelated?

npx --max_old_space_size=2048 webpack --config /builddir/build/BUILD/katello-4.12.0.pre.master/usr/share/foreman/config/webpack.config.js --bail --env pluginName=katello
[webpack-cli] ModuleNotFoundError: Module not found: Error: Can't resolve 'dequal' in '/usr/lib/node_modules/use-deep-compare-effect/dist'
    at /usr/lib/node_modules/webpack/lib/Compilation.js:2016:28
    at /usr/lib/node_modules/webpack/lib/NormalModuleFactory.js:798:13
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:10:1)
    at /usr/lib/node_modules/webpack/lib/NormalModuleFactory.js:270:22
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:9:1)
    at /usr/lib/node_modules/webpack/lib/NormalModuleFactory.js:434:22
    at /usr/lib/node_modules/webpack/lib/NormalModuleFactory.js:116:11
    at /usr/lib/node_modules/webpack/lib/NormalModuleFactory.js:670:25
    at /usr/lib/node_modules/webpack/lib/NormalModuleFactory.js:855:8
    at /usr/lib/node_modules/webpack/lib/NormalModuleFactory.js:975:5
    at /usr/lib/node_modules/webpack/node_modules/neo-async/async.js:6883:13
    at /usr/lib/node_modules/webpack/lib/NormalModuleFactory.js:958:45
    at finishWithoutResolve (/usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:372:11)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:461:15
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:519:5
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:15:1)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:519:5
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:27:1)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/DescriptionFilePlugin.js:89:43
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:519:5
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:16:1)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/forEachBail.js:39:13
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/AliasPlugin.js:148:14
    at next (/usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/forEachBail.js:35:3)
    at forEachBail (/usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/forEachBail.js:49:9)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/AliasPlugin.js:61:5
    at _next0 (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:8:1)
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:30:1)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:519:5
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:16:1)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:519:5
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:16:1)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:519:5
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:27:1)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/DescriptionFilePlugin.js:89:43
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:519:5
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:16:1)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:519:5
    at eval (eval at create (/usr/lib/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:15:1)
    at /usr/lib/node_modules/webpack/node_modules/enhanced-resolve/lib/DirectoryExistsPlugin.js:41:15
    at processTicksAndRejections (internal/process/task_queues.js:81:21)
resolve 'dequal' in '/usr/lib/node_modules/use-deep-compare-effect/dist'
  Parsed request is a module
  using description file: /usr/lib/node_modules/use-deep-compare-effect/package.json (relative path: ./dist)
    Field 'browser' doesn't contain a valid alias configuration
    resolve as module
      looking for modules in /builddir/build/BUILD/katello-4.12.0.pre.master/usr/share/foreman/node_modules
        single file module
          using description file: /builddir/build/BUILD/katello-4.12.0.pre.master/package.json (relative path: ./usr/share/foreman/node_modules/dequal)
            no extension
              Field 'browser' doesn't contain a valid alias configuration
              /builddir/build/BUILD/katello-4.12.0.pre.master/usr/share/foreman/node_modules/dequal doesn't exist
            .js
              Field 'browser' doesn't contain a valid alias configuration
              /builddir/build/BUILD/katello-4.12.0.pre.master/usr/share/foreman/node_modules/dequal.js doesn't exist
            .json
              Field 'browser' doesn't contain a valid alias configuration
              /builddir/build/BUILD/katello-4.12.0.pre.master/usr/share/foreman/node_modules/dequal.json doesn't exist
            .wasm
              Field 'browser' doesn't contain a valid alias configuration
              /builddir/build/BUILD/katello-4.12.0.pre.master/usr/share/foreman/node_modules/dequal.wasm doesn't exist
        /builddir/build/BUILD/katello-4.12.0.pre.master/usr/share/foreman/node_modules/dequal doesn't exist
      looking for modules in /builddir/build/BUILDROOT/rubygem-katello-4.12.0-0.1.pre.master.20240126211203415132.pr10867.5277.g3b8f987a32.el8.x86_64/usr/share/gems/gems/katello-4.12.0.pre.master/node_modules
        single file module
          using description file: /builddir/build/BUILDROOT/rubygem-katello-4.12.0-0.1.pre.master.20240126211203415132.pr10867.5277.g3b8f987a32.el8.x86_64/usr/share/gems/gems/katello-4.12.0.pre.master/package.json (relative path: ./node_modules/dequal)
            no extension
              Field 'browser' doesn't contain a valid alias configuration
              /builddir/build/BUILDROOT/rubygem-katello-4.12.0-0.1.pre.master.20240126211203415132.pr10867.5277.g3b8f987a32.el8.x86_64/usr/share/gems/gems/katello-4.12.0.pre.master/node_modules/dequal doesn't exist
            .js
              Field 'browser' doesn't contain a valid alias configuration
              /builddir/build/BUILDROOT/rubygem-katello-4.12.0-0.1.pre.master.20240126211203415132.pr10867.5277.g3b8f987a32.el8.x86_64/usr/share/gems/gems/katello-4.12.0.pre.master/node_modules/dequal.js doesn't exist
            .json
              Field 'browser' doesn't contain a valid alias configuration
              /builddir/build/BUILDROOT/rubygem-katello-4.12.0-0.1.pre.master.20240126211203415132.pr10867.5277.g3b8f987a32.el8.x86_64/usr/share/gems/gems/katello-4.12.0.pre.master/node_modules/dequal.json doesn't exist
            .wasm
              Field 'browser' doesn't contain a valid alias configuration
              /builddir/build/BUILDROOT/rubygem-katello-4.12.0-0.1.pre.master.20240126211203415132.pr10867.5277.g3b8f987a32.el8.x86_64/usr/share/gems/gems/katello-4.12.0.pre.master/node_modules/dequal.wasm doesn't exist
        /builddir/build/BUILDROOT/rubygem-katello-4.12.0-0.1.pre.master.20240126211203415132.pr10867.5277.g3b8f987a32.el8.x86_64/usr/share/gems/gems/katello-4.12.0.pre.master/node_modules/dequal doesn't exist
rake aborted!
Command failed with status (2): [npx --max_old_space_size=2048 webpack --co...]

That said, nodejs-downshift noarch 5.4.7-1.el8 http_yum_theforeman_org_katello_nightly_katello_el8_x86_64 282 k was still installed as an RPM, so question for those who understand packit better: is it still useful as a test in this circumstance?

@ekohl
Copy link
Member

ekohl commented Jan 29, 2024

https://community.theforeman.org/t/webpack-5-merge-this-friday-2024-01-26/36581/6 mentions dequal

@ehelms
Copy link
Member Author

ehelms commented Jan 29, 2024

That said, nodejs-downshift noarch 5.4.7-1.el8 http_yum_theforeman_org_katello_nightly_katello_el8_x86_64 282 k was still installed as an RPM, so question for those who understand packit better: is it still useful as a test in this circumstance?

No, the spec file will get an update after this change gets merged.

@ehelms
Copy link
Member Author

ehelms commented Feb 2, 2024

Any takers on review?

Copy link
Contributor

@wbclark wbclark left a comment

Choose a reason for hiding this comment

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

No, the spec file will get an update after this change gets merged.

it was a dumb question, but what I meant was, does packit give us a valid test of the effects of removing this dependency, given that it's still available here as an RPM. but then I thought about the fact that just because it's present doesn't mean katello will try to use it.

Any takers on review?

sure, ACK

Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

ACK, thanks @ekohl and @ehelms

@chris1984 chris1984 merged commit 383e45a into Katello:master Feb 5, 2024
5 of 6 checks passed
@chris1984 chris1984 self-assigned this Feb 5, 2024
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.

4 participants