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.

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

Fix the error could not not resolve module "node:os" from cosmiconfig preventing to build the UI #3987

Merged
merged 1 commit into from
Sep 5, 2023
Merged

Fix the error could not not resolve module "node:os" from cosmiconfig preventing to build the UI #3987

merged 1 commit into from
Sep 5, 2023

Conversation

opudrovs
Copy link
Contributor

@opudrovs opudrovs commented Sep 4, 2023

Closes #3986

Fixes the following error preventing to build the UI:

[go-build 13/13] RUN --mount=type=cache,target=/root/.cache/go-build LDFLAGS=${LDFLAGS##-X localbuild=true} GIT_COMMIT=$GIT_COMMIT make gitops-server [cached]
       → yarn build
       → yarn run v1.22.19
       → warning package.json: No license field
       → $ parcel build --target default
       → Building...
       → Bundling...
       → Packaging & Optimizing...
       → 🚨 Build failed.
       → 
       → @parcel/optimizer-htmlnano: Could not resolve module "node:os" from 
       → "/home/app/node_modules/cosmiconfig/dist/index.js"
       → 
       → 
       →   Error: Could not resolve module "node:os" from 
       →   "/home/app/node_modules/cosmiconfig/dist/index.js"
       →       at NodePackageManager.resolveInternal 
       →   (/home/app/node_modules/@parcel/package-manager/lib/index.js:3672:21)
       →       at NodePackageManager.resolveSync 
       →   (/home/app/node_modules/@parcel/package-manager/lib/index.js:3567:29)
       →       at NodePackageManager.requireSync 
       →   (/home/app/node_modules/@parcel/package-manager/lib/index.js:3394:33)
       →       at Module.m.require 
       →   (/home/app/node_modules/@parcel/package-manager/lib/index.js:3410:25)
       →       at require (node:internal/modules/cjs/helpers:119:18)
       →       at Object.<anonymous> 
       →   (/home/app/node_modules/cosmiconfig/dist/index.js:23:35)
       →       at Module._compile (node:internal/modules/cjs/loader:1198:14)
       →       at Object.Module._extensions..js 
       →   (node:internal/modules/cjs/loader:1252:10)
       →       at Module.load (node:internal/modules/cjs/loader:1076:32)
       →       at NodePackageManager.load 
       →   (/home/app/node_modules/@parcel/package-manager/lib/index.js:3423:15)
       → info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
       → error Command failed with exit code 1.
       → make: *** [Makefile:165: ui] Error 1
     
     ERROR IN: [ui 12/12] RUN --mount=type=cache,target=/home/app/ui/.parcel-cache make ui

Build Failed: ImageBuild: executor failed running [/bin/sh -c make ui]: exit code: 2

based on a suggestion from the following issue:
parcel-bundler/parcel#9224

@opudrovs opudrovs added the bug Something isn't working label Sep 4, 2023
foot
foot reviewed Sep 4, 2023
View reviewed changes
website/package.json Outdated
@@ -50,6 +50,7 @@
"yarn-audit-fix": "^9.2.1"
},
"resolutions": {
"htmlnano": "2.0.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think we need it here, the website (docusaurus) doesn't seem to use htmlnano

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good point, will remove it and test the docs.

foot
foot reviewed Sep 4, 2023
View reviewed changes
package.json
@@ -117,5 +117,8 @@
},
"alias": {
"yaml": "yaml/browser/dist/index.js"
},
"resolutions": {
"htmlnano": "2.0.3"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does running yarn locally give some changes to the yarn.lock we should add too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add it, the PR is not up for review yet, I was just testing the main fix. Will submit it for review tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated yarn.lock. To the final testing now.

@foot
Copy link
Contributor

foot commented Sep 4, 2023

  • Can only repro w/ tilt up
  • Not with any combination of rm -rf node_modules / make node_modules etc locally on MacOS
  • Fix here works!

Nice one!

@opudrovs
Copy link
Contributor Author

opudrovs commented Sep 4, 2023 •
edited
Loading

@foot thanks for reproducing the error and checking the fix! It's great that it works as expected. I am polishing the PR now and will raise it probably even today.

@opudrovs opudrovs force-pushed the 3986-fix-cosmiconfig-error branch 2 times, most recently from 7d2e73e to a72ef7f Compare September 4, 2023 15:49
@opudrovs
Copy link
Contributor Author

opudrovs commented Sep 4, 2023 •
edited
Loading

Tested the app locally with Tilt and also with yarn start in addition to Tilt, it works fine for me with the fix.

Also confirmed that the website does not need this fix indeed.

@opudrovs opudrovs marked this pull request as ready for review September 4, 2023 17:07
@opudrovs opudrovs requested review from foot and jpellizzari September 4, 2023 17:07
@opudrovs opudrovs force-pushed the 3986-fix-cosmiconfig-error branch 2 times, most recently from 9c18623 to 6db93d4 Compare September 5, 2023 10:20
@opudrovs
Downgrade the htmlnano package to fix error `could not not resolve …
6506857 
…module "node:os"` in the `cosmiconfig` package.

Update the `yarn.lock` file.
@opudrovs opudrovs force-pushed the 3986-fix-cosmiconfig-error branch from 6db93d4 to 6506857 Compare September 5, 2023 10:51
@jpellizzari
Copy link
Contributor

@opudrovs Any idea what htmlnano and cosmicconfig are used for? A require("node:os") means it is looking for a newer version of node.

I'm not familiar with either of these packages.

@opudrovs
Copy link
Contributor Author

opudrovs commented Sep 5, 2023 •
edited
Loading

@jpellizzari it looks like htmlnano and cosmiconfig are Parcel's subdependencies, they are not imported by the project directly:

olga@MacBook-Pro weave-gitops % npm list htmlnano
@weaveworks/[email protected] /Volumes/Macintosh_HD_Data/_job_ww/_src/weave-gitops
└─┬ @parcel/[email protected]
  └─┬ @parcel/[email protected]
    └── [email protected]

olga@MacBook-Pro weave-gitops % npm list cosmiconfig 
@weaveworks/[email protected] /Volumes/Macintosh_HD_Data/_job_ww/_src/weave-gitops
└─┬ @parcel/[email protected]
  └─┬ @parcel/[email protected]
    └─┬ [email protected]
      └── [email protected]

@opudrovs opudrovs merged commit 0f0d663 into main Sep 5, 2023
19 checks passed
@opudrovs opudrovs deleted the 3986-fix-cosmiconfig-error branch September 5, 2023 15:54
@weave-gitops-bot weave-gitops-bot mentioned this pull request Sep 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpellizzari jpellizzari jpellizzari approved these changes

@foot foot Awaiting requested review from foot

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error could not not resolve module "node:os" from cosmiconfig prevents to build the UI
3 participants
@opudrovs @foot @jpellizzari