Circular dependencies / require cycle

[isaachinman]

Describe the bug

Surprised this has not been reported yet. I've used Orama previously with Vite as a bundler without issues.

I'm now setting up an Expo web/native project, and see these warnings:

 WARN  Require cycle: ../../../node_modules/@orama/orama/dist/esm/errors.js -> ../../../node_modules/@orama/orama/dist/esm/utils.js -> ../../../node_modules/@orama/orama/dist/esm/errors.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: ../../../node_modules/@orama/orama/dist/esm/methods/search.js -> ../../../node_modules/@orama/orama/dist/esm/methods/search-fulltext.js -> ../../../node_modules/@orama/orama/dist/esm/methods/search.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: ../../../node_modules/@orama/orama/dist/esm/methods/search.js -> ../../../node_modules/@orama/orama/dist/esm/methods/search-hybrid.js -> ../../../node_modules/@orama/orama/dist/esm/methods/search.js

These are not benign, it actually resulted in a fetchDocuments is not a function runtime error when calling the search function.

At the moment, the only workaround I can think of would be for users to bundle the entirety of the Orama source into a single file via esbuild or similar.

To Reproduce

1. npx create-expo-app@latest test
2. cd test && npm i @orama/orama
3. npm run ios

Expected behavior

No require cycles – shared dependencies should be extracted to a third file to break the circularity.

Environment Info

N/A

Affected areas

Search

Additional context

Edit: Seems as though this is not the first time: #181

[isaachinman]

While this error was harmless with Metro+iOS, it actually causes a fatal error with Metro+web:

fetchDocuments is not a function

So far, the only workaround I have found is to prebundle the @orama/orama source via esbuild and commit it to my vc. However you are compiling and distributing your code needs to change.

[isaachinman]

@micheleriva Any chance you would consider rollup, and export to a single file?

[micheleriva]

This is a very odd problem - never had it before. We had so much trouble with bundling, mainly due to ESM <> CJS incompatibilities in Node.js. That's the main reason why we never used Rollup or ESBuild - we couldn't get the final compiled files to work correctly in CJS. I'm wondering if we could just create a new, separate bundle we can ship which is basically an ESM bundled with ESBuild and you could import it. But it's still strange how we never faced this problem before

[isaachinman]

I'm also a bit surprised that no RN users have come along and reported this yet. The main culprit is Metro, which is unbelievably bad but continues to exist for various historical reasons.

Note that the circularity is harmless on actual native RN due to platform differences, but becomes a fatal error when exporting for web via Expo/react-native-web.

I've already patched this in my own app via esbuild as I've said, but if you're open to fixing the issue I can provide a minimal repro tomorrow 🙏

[micheleriva]

Yes I'm open to fixing this and I'd appreciate a repro! Also, if you'd send a PR that would be even better 🙏

[isaachinman]

Here's the minimal repro:

https://github.com/isaachinman/orama-require-cycle-reproduction

Just clone and run bun run web and you will see the error: fetchDocuments is not a function.

Note that if you remove experimentalImportSupport, the circularity does not cause a fatal error even on web. However, Expo's new treeshaking can reduce Metro bundles by ~50% in some cases, in my testing. In my real-world application, I have 20+ external deps, and Orama is the only dependency that fails to compile with experimentalImportSupport on.

[isaachinman]

@micheleriva I am not familiar with pnpm or tshy, but I put together an example PR:

#963

Not sure how the core maintainers want to implement this – I assume you do not want to touch existing entry points so as to not introduce new regressions.

I can confirm that the approach in the PR fixes the fetchDocuments is not a function error.

Another (perhaps simpler) approach would be to actually remove the circularity from the source code. I am not familiar enough with the code base to know how feasible that is, but that would solve the issue without needing to introduce a flattened/single-file entry point.