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

Circular dependency between fs_core.ts <-> fs_fd.ts #58

Closed
kateinoigakukun opened this issue Dec 6, 2023 · 1 comment · Fixed by #60
Closed

Circular dependency between fs_core.ts <-> fs_fd.ts #58

kateinoigakukun opened this issue Dec 6, 2023 · 1 comment · Fixed by #60

Comments

@kateinoigakukun
Copy link
Contributor

Currently fs_core.ts and fs_fd.ts import each other. Fortunately it does not cause any problem at the moment, but Rollup conservatively warns about this. (I faced it while changing default bundled WASI implementation of ruby.wasm: ruby/ruby.wasm#332)

$ npx rollup ./dist/index.js -o /dev/null
./dist/index.js → ../../../../../../dev/null...
(!) Circular dependency
dist/fs_core.js -> dist/fs_fd.js -> dist/fs_core.js
created ../../../../../../dev/null in 174ms

There are some options to address this issue. Which option do you like, @bjorn3? I personally prefer 2. since it is the simplest and does not change any public API.

  1. Just ignore the warning or fix rollup to produce more precise cycle detection
  2. Merge fs_core.ts and fs_fd.ts
  3. Re-organize fs_core.ts and fs_fd.ts's API to avoid cycle
@bjorn3
Copy link
Owner

bjorn3 commented Dec 13, 2023

Forgot to reply here. I think merging fs_core.ts and fs_fd.ts is fine. Or maybe separate files and directories instead. There shouldn't be a cyclic dependency between those except maybe through type hints.

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 a pull request may close this issue.

2 participants