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

Fatal error in Node due to named import from uuid library #1139

Closed
arpowers opened this issue May 6, 2023 · 8 comments · Fixed by #1142
Closed

Fatal error in Node due to named import from uuid library #1139

arpowers opened this issue May 6, 2023 · 8 comments · Fixed by #1142

Comments

@arpowers
Copy link

arpowers commented May 6, 2023

Appears you guys are requesting a named export from a commonjs module: uuid

in this file:
https://github.com/hwchase17/langchainjs/blob/main/langchain/src/vectorstores/pinecone.ts

on my dev setup, which uses Vite and standard ESM tooling:

Named export 'v4' not found. The requested module 'uuid' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'uuid';
const { v4: uuidv4 } = pkg;


  import { v4 as uuidv4 } from "uuid";
  ^^
  SyntaxError: Named export 'v4' not found. The requested module 'uuid' is a CommonJS module, which may not support all module.exports as named exports.
  CommonJS modules can always be imported via the default export, for example using:

  import pkg from 'uuid';
  const { v4: uuidv4 } = pkg;

  at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
  at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
  at async ModuleJob.run (node:internal/modules/esm/module_job:190:5)

If you're going to use uuid native libs, then i think best way to do this is wildcard import

import * as uuid from 'uuid'

//later: uuid.v4
@arpowers arpowers changed the title ESM issue with uuid library Fatal error in Node due to named import from uuid library May 6, 2023
@nfcampos
Copy link
Collaborator

nfcampos commented May 6, 2023

Hi, that is actually an issue in your local setup, check here https://www.npmjs.com/package/uuid?activeTab=code, look at the file wrapper.mjs, uuid actually exports an ESM file. Also check we actually test against a number of common JS tools with their default configuration, including Vite, https://github.com/hwchase17/langchainjs/blob/main/test-exports-vite/src/chain.ts#L8 where we successfully import a module langchain/callbacks that has a similar import of uuid.

In any case I've updated the uuid imports in #1142

@nfcampos nfcampos linked a pull request May 6, 2023 that will close this issue
@arpowers
Copy link
Author

arpowers commented May 6, 2023

@nfcampos thanks for the fix,

but fyi the issue was that the module was importing uuid from a CommonJS context, from within langchain. As you know, cjs/esm is quite messy sometimes, but on my end, we have 100+ dependencies which work fine....

i couldn't quite figure it out (no known way to debug this), but it was a special case.

@arpowers
Copy link
Author

arpowers commented May 8, 2023

@nfcampos reopen this issue, is related to this bug:
uuidjs/uuid#692

the error is now:

 ERROR  Unexpected token 'export'                                                                                                            10:48:45 AM

  export { default as v1 } from './v1.js';
  ^^^^^^

  SyntaxError: Unexpected token 'export'
  at internalCompileFunction (node:internal/vm:73:18)
  at wrapSafe (node:internal/modules/cjs/loader:1176:20)
  at Module._compile (node:internal/modules/cjs/loader:1218:27)
  at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
  at Module.load (node:internal/modules/cjs/loader:1117:32)
  at Module._load (node:internal/modules/cjs/loader:958:12)
  at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:169:29)
  at ModuleJob.run (node:internal/modules/esm/module_job:194:25)
  at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

@nfcampos
Copy link
Collaborator

nfcampos commented May 8, 2023

That sounds like an issue with your local setup, we test this in CI before every release and it works, if you look through the issue you linked you'll see that the issue is not with uuid (or langchain) but with whatever build tool you're using that doesn't follow the conventions around loading ESM/CJS files. I don't see any way I can do anything here to work around your local setup, but if you have any suggestions let me know.

@arpowers
Copy link
Author

arpowers commented May 8, 2023

@nfcampos not an issue, im simply running esbuild to transpile .ts files... uuid is barely maintained AND has a bug in that it allows its module files to be interpreted as CJS in certain situation as discussed in the bug report on that lib.

@nfcampos
Copy link
Collaborator

nfcampos commented May 8, 2023

@arpowers see here #1170 I've added a test package that bundles with esbuild and it builds successfully. I suggest you compare your esbuild setup to this (which is copied from the uuid issue above) and see where yours differs

@arpowers
Copy link
Author

arpowers commented May 8, 2023

@nfcampos thank you...

ok, i've actually switched to TSX because it uses a custom resolver which works 🤷

its still an edge-case bug with uuid, but also a legacy nodejs issue

for the future of this library, i respectfully suggest considering reducing the amount of sub-dependencies where possible.

that way you don't end up in the same kludge as webpack and similar libs

@nfcampos
Copy link
Collaborator

nfcampos commented May 9, 2023

Yea I agree, we want to have as little required dependencies (optional dependencies are inevitable given the nature of the library, but they're handled in a way where they don't bother people who don't use them), it's on my mind to review/reduce them further

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