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

TypeError: Cannot read properties of undefined (reading 'StringLiteral') #158248

Closed
bpasero opened this issue Aug 16, 2022 · 8 comments
Closed

TypeError: Cannot read properties of undefined (reading 'StringLiteral') #158248

bpasero opened this issue Aug 16, 2022 · 8 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug json JSON support issues

Comments

@bpasero
Copy link
Member

bpasero commented Aug 16, 2022

This just appears in the integration test logs (exthost.log), thought I would report it anyway:

[2022-08-15 23:13:00.217] [exthost] [error] TypeError: Cannot read properties of undefined (reading 'StringLiteral')
    at JSONCompletionItemProvider.isLast (/mnt/vss/_work/1/s/extensions/npm/out/features/jsonContributions.js:131:53)
    at JSONCompletionItemProvider.provideCompletionItems (/mnt/vss/_work/1/s/extensions/npm/out/features/jsonContributions.js:99:33)
    at CompletionsAdapter.provideCompletionItems (file:///mnt/vss/_work/1/s/src/module.ts:938:44)
    at file:///mnt/vss/_work/1/s/src/module.ts:2241:75
    at ExtHostLanguageFeatures._withAdapter (file:///mnt/vss/_work/1/s/src/module.ts:1894:18)
    at ExtHostLanguageFeatures.$provideCompletionItems (file:///mnt/vss/_work/1/s/src/module.ts:2241:15)
    at RPCProtocol._doInvokeHandler (file:///mnt/vss/_work/1/s/src/module.ts:455:17)
    at RPCProtocol._invokeHandler (file:///mnt/vss/_work/1/s/src/module.ts:440:32)
    at RPCProtocol._receiveRequest (file:///mnt/vss/_work/1/s/src/module.ts:366:19)
    at RPCProtocol._receiveOneMessage (file:///mnt/vss/_work/1/s/src/module.ts:296:10)
    at file:///mnt/vss/_work/1/s/src/module.ts:161:42
    at Listener.invoke (file:///mnt/vss/_work/1/s/src/module.ts:622:17)
    at PrivateEventDeliveryQueue.deliver (file:///mnt/vss/_work/1/s/src/module.ts:824:22)
    at Emitter.fire (file:///mnt/vss/_work/1/s/src/module.ts:785:24)
    at BufferedEmitter.fire (file:///mnt/vss/_work/1/s/src/module.ts:644:19)
    at file:///mnt/vss/_work/1/s/src/module.ts:251:22
    at Listener.invoke (file:///mnt/vss/_work/1/s/src/module.ts:622:17)
    at PrivateEventDeliveryQueue.deliver (file:///mnt/vss/_work/1/s/src/module.ts:824:22)
    at Emitter.fire (file:///mnt/vss/_work/1/s/src/module.ts:785:24)
    at BufferedEmitter.fire (file:///mnt/vss/_work/1/s/src/module.ts:644:19)
    at PersistentProtocol._receiveMessage (file:///mnt/vss/_work/1/s/src/module.ts:975:23)
    at file:///mnt/vss/_work/1/s/src/module.ts:848:73
    at Listener.invoke (file:///mnt/vss/_work/1/s/src/module.ts:622:17)
    at PrivateEventDeliveryQueue.deliver (file:///mnt/vss/_work/1/s/src/module.ts:824:22)
    at Emitter.fire (file:///mnt/vss/_work/1/s/src/module.ts:785:24)
    at ProtocolReader.acceptChunk (file:///mnt/vss/_work/1/s/src/module.ts:388:21)
    at file:///mnt/vss/_work/1/s/src/module.ts:344:51
    at Socket.listener (file:///mnt/vss/_work/1/s/src/module.ts:70:4)
    at Socket.emit (node:events:526:28)
    at addChunk (node:internal/streams/readable:315:12)
    at readableAddChunk (node:internal/streams/readable:289:9)
    at Socket.push (node:internal/streams/readable:228:10)
    at Pipe.onStreamRead (node:internal/stream_base_commons:190:23)
@aeschli
Copy link
Contributor

aeschli commented Aug 26, 2022

SyntaxKind.StringLiteral is a const enum, so the value (10) should have been inlined.

@jrieken Are the integration tests built with the new builder?

@jrieken
Copy link
Member

jrieken commented Aug 26, 2022

Yes, there is no more inlining happening for tests. @aeschli this could be hint that you have a cycling dependency that shows when not inlining enums

@aeschli
Copy link
Contributor

aeschli commented Aug 26, 2022

SyntaxKind comes from a d.ts, that has no requires.
There are cyclic requires between jsonContributions <-> packageJSONContribution.ts, but it's only for type declaratons

How do I get more info here?

@jrieken
Copy link
Member

jrieken commented Aug 29, 2022

Hard to say. You can run the transpile-task locally (transpile-extension:<EXT_NAME>) and check the files it has produced. Might be a cyclic dependency or a fundamental problem, might be that the value doesn't exist at runtime because the d-ts-files assumes its consumer always inline everything

@aeschli
Copy link
Contributor

aeschli commented Aug 29, 2022

I removed the cyclic dependency jsonContributions <-> packageJSONContribution, it didn't make a change
The transpiled code looks like that:

const jsonc_parser_1 = require("jsonc-parser");
...
jsonc_parser_1.SyntaxKind.StringLiteral

The transpiler just converts the import to require, without trying to locate the d.ts file or looking at any declarations.

I tested the same when the const enum is in a enum.ts file next to the source file. Same issue if the enum is export declare const enum SyntaxKind as in the d.ts file.
It works if it's export const enum SyntaxKind, because then the transpiler generated the enum in the enum.js file.

So this looks like a fundamental problem with const enums in d.ts files.

Looks like I have to change the jsonc_parser code to compile with preserveConstEnums": true.

@jrieken
Copy link
Member

jrieken commented Aug 29, 2022

Looks like I have to change the jsonc_parser code to compile with preserveConstEnums": true.

Yeah, that might be an unfortunate fact. The transpiler doesn't actually resolve enums and therefore it cannot read/inline its values. It always references them by name and expects something "to be there at runtime". Using the preserveConstEnums -option sounds like the best option

@aeschli
Copy link
Contributor

aeschli commented Aug 30, 2022

I guess it's fair to argue that using const enums in in exported node modules APIs d.ts is not ideal. It will only work for TS, but not when using a library out of JS. I'll make a pass through my node node modules.

Still we will hit this with node modules we can't easly change. I guess in that case we need to redeclare the enum in our land.

Can we add a lint rule that warns about the usage of const enums from d.t.s?

@aeschli
Copy link
Contributor

aeschli commented Dec 7, 2022

Fixed by microsoft/node-jsonc-parser#70

@aeschli aeschli closed this as completed Dec 7, 2022
@aeschli aeschli added json JSON support issues bug Issue identified by VS Code Team member as probable bug labels Dec 7, 2022
@aeschli aeschli added this to the September 2022 milestone Dec 7, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug json JSON support issues
Projects
None yet
Development

No branches or pull requests

3 participants