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

Identifier '$Completions' has already been declared #60

Closed
gajus opened this issue Jan 26, 2024 · 10 comments · Fixed by #85
Closed

Identifier '$Completions' has already been declared #60

gajus opened this issue Jan 26, 2024 · 10 comments · Fixed by #85

Comments

@gajus
Copy link

gajus commented Jan 26, 2024

Getting error:

file:///Users/x/Developer/contra/gaia/node_modules/.pnpm/openai@4.20.0/node_modules/openai/resources/index.mjs?iitm=true:55
    let $Completions = namespace.Completions
        ^

SyntaxError: Identifier '$Completions' has already been declared
    at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:168:18)
    at callTranslator (node:internal/modules/esm/loader:279:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:285:30)

The file contents:

// File generated from our OpenAPI spec by Stainless.
export * from "./chat/index.mjs";
export * from "./shared.mjs";
export { Audio } from "./audio/audio.mjs";
export { Beta } from "./beta/beta.mjs";
export { Completions, } from "./completions.mjs";
export { Embeddings } from "./embeddings.mjs";

After taking a closer look, the problem seems to be that ./chat/index.mjs exports:

// File generated from our OpenAPI spec by Stainless.
export { Chat } from "./chat.mjs";
export { Completions, } from "./completions.mjs";
//# sourceMappingURL=index.mjs.map

which end up conflicting with export { Completions, } from "./completions.mjs";

@gajus
Copy link
Author

gajus commented Jan 26, 2024

Got to this error after tackling #59, for what it is worth.

@jsumners-nr
Copy link
Contributor

#53 was meant to solve this sort of issue. But we seem to be playing whack-a-mole with the ludicrous number of permutations ESM provides for exporting entities. I am quite open to suggestions here.

@bizob2828
Copy link

bizob2828 commented May 2, 2024

It's worth noting this is failing at a higher level now in 4.40.1 of openai. However that release broke cjs exports, and someone logged this issue

npm i openai@4.40.1
node --loader=import-in-the-middle/hook.mjs --input-type=module -e "import OpenAI from 'openai'; const a = new OpenAI();"
    let $OpenAI = $171467968826591576f6c.OpenAI
        ^

SyntaxError: Identifier '$OpenAI' has already been declared
    at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:167:18)
    at callTranslator (node:internal/modules/esm/loader:285:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:291:30)

@jpintoic
Copy link

jpintoic commented May 23, 2024

I'm trying to setup Sentry for Node.js (type: module) with the following guide https://docs.sentry.io/platforms/javascript/guides/express/install/esm/ wich recommends the --import command line option to import the instrument.js/.mjs file and can't at the moment due to this issue. I keep getting the Identifier '$Completions' has already been declared error. It seems they are using this package in their SDK.

@timfish
Copy link
Contributor

timfish commented May 23, 2024

This can be reproduced with:

a.mjs

export const foo = "a"

b.mjs

export const foo = "b"

index.mjs

export * from './a.mjs'
export * from './b.mjs'

But we seem to be playing whack-a-mole with the ludicrous number of permutations ESM provides for exporting entities.

The Map isn't working to de-duplicate because the keys are derived from:

Buffer.from(modFile, 'hex') + Date.now() + randomBytes(4).toString('hex')
image

@jpintoic
Copy link

@timfish, looks promising!

@jsumners-nr
Copy link
Contributor

@timfish what is the expected "export" of:

index.mjs

export * from './a.mjs'
export * from './b.mjs'

Genuinely, I do not know what the export should be. Both a.mjs and b.mjs export a single foo symbol. So which one wins, b?

@timfish
Copy link
Contributor

timfish commented May 28, 2024

Ok so I just tested Node without the loader and the behaviour depends on the import:

With:
a.mjs

export const foo = "a"

b.mjs

export const foo = "b"

duplicate.mjs

export * from './a.mjs'
export * from './b.mjs'

The following:

import * as e from './duplciate.mjs'

console.log('exports', e);

Results in:

exports [Module: null prototype] {  }

And this

import { foo } from './duplciate.mjs'

console.log('foo', foo);

Results in an error

import { foo } from '../fixtures/duplicate.mjs'
         ^^^
SyntaxError: The requested module '../fixtures/duplicate.mjs' contains conflicting star exports for name 'foo'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:131:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:213:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at async loadESM (node:internal/process/esm_loader:34:7)
    at async handleMainPromise (node:internal/modules/run_main:66:12)

So I guess we need to mirror this behaviour?

@jsumners-nr
Copy link
Contributor

That makes sense for the presented replication. I think we should definitely throw an error in this case since it is clearly an unresolvable situation. However, we need to figure out how to replicate the allowed graph that the OpenAI module seems to be making use of, and determine a way to handle it appropriately.

The whack-a-mole game continues 🤣

@timfish
Copy link
Contributor

timfish commented May 28, 2024

Throwing is ok in some circumstances, but we'd rather not have any cases where registering the hook causes code to throw where it didn't before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants