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

Update queue type to avoid build error #673

Merged
merged 2 commits into from
Apr 8, 2023

Conversation

ashburnham
Copy link
Contributor

Existing code gives this error

node_modules/langchain/dist/util/async_caller.d.ts:30:22 - error TS2702: 'PQueueMod' only refers to a type, but is being used as a namespace here.
30 protected queue: PQueueMod.default;

This PR fixes it..

Existing code gives this error

node_modules/langchain/dist/util/async_caller.d.ts:30:22 - error TS2702: 'PQueueMod' only refers to a type, but is being used as a namespace here.
30     protected queue: PQueueMod.default;

This PR fixes it..
@vercel
Copy link

vercel bot commented Apr 8, 2023

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

1 Ignored Deployment
Name Status Preview Updated (UTC)
langchainjs-docs ⬜️ Ignored (Inspect) Apr 8, 2023 2:32pm

@nfcampos
Copy link
Collaborator

nfcampos commented Apr 8, 2023

@ashburnham see the "Build and check types" CI job result, this appears to make the build fail on CI (and in my laptop). Can you share more about what's different about your environment

@nfcampos nfcampos added the question Further information is requested label Apr 8, 2023
@ashburnham
Copy link
Contributor Author

I'm using it in a commonjs module (which I had due to other dependencies. My tsconfig is as follows:

{
  "include": [
    "src/**/*",
  ],
  "compilerOptions": {
    "target": "ES2020",           /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019' or 'ESNEXT'. */
    "module": "commonjs",         /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', or 'ESNext'. */
    "outDir": "./dist",           /* Redirect output structure to the directory. */
    "strict": true,              /* Enable all strict type-checking options. */
    "baseUrl": "./src",
    "typeRoots": [
      "node_modules/@types"
    ],                            /* List of folders to include type definitions from. */
    "types": [
      "node",
      "jest"
    ],
    "lib":  [
      "ES2019",
      "dom",
    ], /* Type declaration files to be included in compilation. */
    "esModuleInterop": true,      /* Enables emit interoperability between CommonJS and ES Modules via creation of namespace objects for all imports. Implies 'allowSyntheticDefaultImports'. */
    "inlineSourceMap": true,
    "resolveJsonModule": true,
    "moduleResolution": "node10",
    "allowJs": true
  }
}

It took me a while to get it does so that this is the only error left, and it seems it was as my PR suggests only 3 days ago... so I was hoping it could be changed back. Alternatively, any other workaround to make it work in a commonjs project would be very helpful! 🙏🙏🙏🙏

p.s. I've been followinf this issue, which is related, but none of it works for me.

@ashburnham
Copy link
Contributor Author

p.s. I'm going skipLibCheck on langchain for now, but that's not a long-term solution...

@nfcampos
Copy link
Collaborator

nfcampos commented Apr 8, 2023

I've committed an alternative here which I think fixes it, but in testing your tsconfig it seems to be invalid, eg. "moduleResolution": "node10", doesn't exist according to the docs. I'd usually recommend adding skipLibCheck on your projects, you don't gain anything in type checking your dependencies (ie. it doesn't make your own application any more type safe), which were most likely designed for a different tsconfig than you are using, hence the type "errors" you then see

@nfcampos nfcampos merged commit 90afe93 into langchain-ai:main Apr 8, 2023
Copy link
Contributor Author

@ashburnham ashburnham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good solution! Thanks!

@nfcampos
Copy link
Collaborator

nfcampos commented Apr 8, 2023

This was released on 0.0.51, please try again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants