fix(media): no require in mjs modules

[hassiebp]

Important

Replace require with dynamic import for fs and crypto in LangfuseMedia.ts and update tests for compatibility.

• Imports:
  • Replace require with dynamic import for fs and crypto in LangfuseMedia.ts.
  • Use Promise.all for asynchronous imports.
• Error Handling:
  • Log error if fs or crypto modules fail to load.
• Functionality:
  • Update contentSha256Hash method in LangfuseMedia to use dynamically imported crypto.
• Tests:
  • Update test in langfuse-node.spec.ts to expect a minimum number of log calls.

^{This description was created by for fce8a79. It will automatically update as commits are pushed.}

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
langfuse-js	✅ Ready (Inspect)	Visit Preview	Nov 28, 2024 0:17am

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

Changes the module loading strategy for 'fs' and 'crypto' modules in LangfuseMedia.ts from synchronous require() to asynchronous dynamic imports to improve compatibility with ES modules.

• Implements dynamic imports using Promise.all([import("fs"), import("crypto")]) in Node environments
• Potential race condition risk if fs/crypto methods are called before imports complete
• Removed Edge Runtime/Cloudflare Workers specific code path for simpler implementation
• Added error handling for failed module imports via catch block

_{1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

No major changes found since last review. The code remains focused on the dynamic import implementation for fs and crypto modules as previously described, with no significant modifications since the last review.

_{1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Updated test timing checks in langfuse-node.spec.ts to accommodate potential delays from the new dynamic import implementation in LangfuseMedia.ts.

• Modified timing assertions in /langfuse-node/test/langfuse-node.spec.ts to use more flexible comparisons for dynamic import latency
• Potential minor performance impact from async module loading in Node.js environments

Note: This is a focused summary of only the most recent changes, building on previous reviews that covered the core dynamic import implementation.

_{1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Added error logging for dynamic imports and improved test coverage for module loading failures in LangfuseMedia.ts.

• Added error logging in catch block for dynamic imports of fs/crypto modules
• Updated test cases in langfuse-core/test/langfuse.init.spec.ts to verify error handling for module import failures
• Fixed potential race condition by checking module availability before using fs/crypto methods
• Improved error messaging for hash calculation failures when crypto module is unavailable

_{1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Added wrapper function for dynamic imports to prevent bundler issues and improved error handling in LangfuseMedia.ts.

• Added dynamicImport wrapper function to prevent bundlers from incorrectly resolving imports as static
• Improved error handling in contentSha256Hash method with explicit console error when crypto is unavailable
• Added comment explaining why wrapper is needed for bundler compatibility

_{1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Added webpackIgnore comment and improved dynamic import handling in LangfuseMedia.ts to prevent bundler issues with fs/crypto modules.

• Added /* webpackIgnore: true */ comment to prevent incorrect static resolution of dynamic imports
• Improved error handling in contentSha256Hash with explicit console error for missing crypto module
• Wrapped dynamic imports in helper function to maintain browser environment compatibility

_{1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the provided files and context, I'll summarize the most recent changes to the test suite in langfuse-node.spec.ts:

Modified test expectations in langfuse-node.spec.ts to handle variable timing from dynamic imports and async operations.

• Changed flush call expectation from exact count to toBeGreaterThanOrEqual(5) to accommodate timing variations
• Added explicit test for minimum 15 total log calls to verify core functionality
• Updated shutdown test to verify exactly 4 flush calls during cleanup phase
• Improved test reliability by using real timers for async operation verification

The changes make the tests more resilient to timing variations while maintaining strong verification of core functionality.

_{1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes, I'll summarize the most recent modifications to the test suite:

Updated test assertions in langfuse-node.spec.ts to better handle asynchronous behavior and dynamic imports.

• Replaced exact match assertions with toBeGreaterThanOrEqual for flush call counts to handle timing variations
• Added test verification for minimum 10 trace-create log entries to ensure core functionality
• Updated shutdown test to use real timers for accurate async operation testing
• Improved test cleanup by adding explicit shutdownAsync call in afterEach block

_{1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile}