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

build: Hoisting rollup and esbuild optional dependencies #562

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

bmingles
Copy link
Contributor

@bmingles bmingles commented Jun 17, 2024

Rollup and esbuild optional dependencies were removed from the package-lock by this PR: #502

Adding them back to top level optionalDependencies to ensure they are available

For reference to get this list I:

  1. used vscode file history to find the package-lock.json diff that removed the dependencies.
  2. copy / pasted the removed blocks of esbuild and rollup dependencies into a new .json file
  3. Used jq to flatten the names + versions cat myfile.json | jq 'with_entries(.value = .value.version)'
  4. Tweak / copy / pasted into optionalDependencies

Also, in case we need something like this again, jq can be used to find things like:

cat package-lock.json | \
 jq .packages | \
 jq 'with_entries(select(.key | startswith("node_modules")))' | \
 jq 'with_entries(select(.key | contains("darwin-arm64")))' | \
 jq 'with_entries(.value = .value.version)'

UPDATE: Looks like git history can be searched via git log -S for a specific term to determine which package-lock change caused the issue:
e.g.

git log -S esbuild-darwin-64

resolves #560

@bmingles bmingles requested a review from mattrunyon June 17, 2024 21:19
Copy link
Contributor

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

Did you use something to extract this list or just find the entries in package-lock manually? Either way can you add what you did to the PR body so if this happens again in the future we might find this PR and have the instructions

Copy link
Contributor

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

Probably want to add these as well. Not sure we currently use them in plugins, but they're a Vite dependency (I believe for Vite dev mode).

I found these by just searching darwin-arm64": " in package-lock.json which should find anywhere that another package lists something ending in that name in the deps/optional deps.

"@swc/core-darwin-arm64": "1.3.99",
"@swc/core-darwin-x64": "1.3.99",
"@swc/core-linux-arm64-gnu": "1.3.99",
"@swc/core-linux-arm64-musl": "1.3.99",
"@swc/core-linux-x64-gnu": "1.3.99",
"@swc/core-linux-x64-musl": "1.3.99",
"@swc/core-win32-arm64-msvc": "1.3.99",
"@swc/core-win32-ia32-msvc": "1.3.99",
"@swc/core-win32-x64-msvc": "1.3.99"

@mattrunyon
Copy link
Contributor

And if you wanted the nx optionals which shouldn't be needed w/ useNx: false

"@nrwl/nx-darwin-arm64": "15.9.2",
"@nrwl/nx-darwin-x64": "15.9.2",
"@nrwl/nx-linux-arm-gnueabihf": "15.9.2",
"@nrwl/nx-linux-arm64-gnu": "15.9.2",
"@nrwl/nx-linux-arm64-musl": "15.9.2",
"@nrwl/nx-linux-x64-gnu": "15.9.2",
"@nrwl/nx-linux-x64-musl": "15.9.2",
"@nrwl/nx-win32-arm64-msvc": "15.9.2",
"@nrwl/nx-win32-x64-msvc": "15.9.2"

@bmingles bmingles requested a review from mattrunyon June 17, 2024 21:45
@bmingles bmingles merged commit 4d005d2 into deephaven:main Jun 17, 2024
9 checks passed
@bmingles bmingles deleted the 560-optional-rollup branch June 17, 2024 21:53
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.

Remove nx dependency
2 participants