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

Add optional peer dependency in @parcel/rust #10101

Open
wants to merge 1 commit into
base: v2
Choose a base branch
from

Conversation

fengzilong
Copy link

@fengzilong fengzilong commented Mar 8, 2025

↪️ Pull Request

Add missing dependencies napi-wasm for @parcel/rust which is used in packages/core/rust/browser.js

const {Environment, napi} = require('napi-wasm');

Previously napi-wasm was put in devDependencies, this PR moves it to dependencies

💻 Examples

The original issue is that when I try to bundle @parcel/fs in a browser environment like parcel repl did, it says "Could not resolve napi-wasm" since napi-wasm is only declared in devDependencies

🚨 Test instructions

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@devongovett
Copy link
Member

Hmm I think we wanted to avoid installing an extra dependency that isn't needed in most cases, except if you're doing a browser build. Maybe it should be some kind of optional peer dependency or something?

@fengzilong fengzilong force-pushed the fix/rust-dependencies branch from 7bab4c8 to 03d0ede Compare March 9, 2025 11:01
@fengzilong
Copy link
Author

Sounds reasonable to mark it as optional peer dependency. Code has been updated.

@fengzilong fengzilong changed the title Add missing dependencies for @parcel/rust Add optional peer dependency in @parcel/rust Mar 9, 2025
@fengzilong fengzilong force-pushed the fix/rust-dependencies branch from 03d0ede to 5f3ce78 Compare March 10, 2025 12:36
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.

2 participants