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

Named range tuple incompatible with JSII compilation #393

Closed
xXPOLYGONXx opened this issue May 18, 2022 · 5 comments
Closed

Named range tuple incompatible with JSII compilation #393

xXPOLYGONXx opened this issue May 18, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@xXPOLYGONXx
Copy link

Describe the bug
I am using the yaml package as a dev dependency in a JSII Project. Since the implementation of #385 there are errors when trying to compile the project which clearly point to the changes made in named pull request:

[2022-05-18T15:14:02.582] [ERROR] jsii/compiler - Compilation errors prevented the JSII assembly from being created

148 | node_modules/yaml/dist/nodes/Node.d.ts:15:30 - error TS2304: Cannot find name 'start'.
149 |  
150 | 15 export declare type Range = [start: number, valueEnd: number, nodeEnd: number];
151 | ~~~~~
152 | node_modules/yaml/dist/nodes/Node.d.ts:15:35 - error TS1005: ',' expected.
153 |  
154 | 15 export declare type Range = [start: number, valueEnd: number, nodeEnd: number];
155 | ~
156 | node_modules/yaml/dist/nodes/Node.d.ts:15:45 - error TS2304: Cannot find name 'valueEnd'.
157 |  
158 | 15 export declare type Range = [start: number, valueEnd: number, nodeEnd: number];
159 | ~~~~~~~~
160 | node_modules/yaml/dist/nodes/Node.d.ts:15:53 - error TS1005: ',' expected.
161 |  
162 | 15 export declare type Range = [start: number, valueEnd: number, nodeEnd: number];
163 | ~
164 | node_modules/yaml/dist/nodes/Node.d.ts:15:63 - error TS2304: Cannot find name 'nodeEnd'.
165 |  
166 | 15 export declare type Range = [start: number, valueEnd: number, nodeEnd: number];
167 | ~~~~~~~
168 | node_modules/yaml/dist/nodes/Node.d.ts:15:70 - error TS1005: ',' expected.
169 |  
170 | 15 export declare type Range = [start: number, valueEnd: number, nodeEnd: number];

To Reproduce
Create a projen AwsCdkConstructLibrary and try to compile it with the yaml package in the devDependencies.
There might be a easier way to reproduce this issue, but thats how I stumbled on it.

Expected behaviour
It should compile without errors.

Versions (please complete the following information):

  • Environment: Node.js 14.17.2
  • yaml: 2.1.0

Additional context

@xXPOLYGONXx xXPOLYGONXx added the bug Something isn't working label May 18, 2022
@eemeli
Copy link
Owner

eemeli commented May 18, 2022

What's the TypeScript version you're using this with? Named tuples were introduced in its version 4.0 (released in August 2020), so if you're on an earlier version, upgrading to 4.x might fix the issue.

@rgoltz
Copy link

rgoltz commented May 29, 2022

Hi @eemeli - We facing the same issue, also within a AWS Pipeline. Based on my 1st cross-check, we are using a current version of TypeScript.

I assume it's related to JSII itself: Seems that JSII is not able to "transform" this new tuple impl.

Currently we pinned yaml package to 2.0.1 to workaround this issue - This idea doesn't seems a smart idea.

@eemeli
Copy link
Owner

eemeli commented May 29, 2022

@xXPOLYGONXx @rgoltz Please see if yaml@2.1.1 fixes these issues for you? I've there reverted the change from #385, and added a CI test suite currently covering TS 3.8, 4.0, 4.2, and 4.7. I'm not absolutely certain that all of those are fully supported, but at least I've no indication against that atm.

@xXPOLYGONXx
Copy link
Author

@eemeli excuse the late reply. We are using Typescript "^4.6.3" so it should be new enough.
I've tested the version 2.1.1 which you published and it worked without problems. So I would assume that JSII just doesn't support this Typescript feature. Also they stated that they dont support normal Tuples too, which is a bit strange, since without using the naming in tuples the library compiles fine (JSII TypeSystem). Maybe it is converting them to another Datatype which does not work with named tuples.

@eemeli
Copy link
Owner

eemeli commented May 30, 2022

Great! Sounds like this is solved, at least for now.

I've realized through this exercise that it's not clear at all what an appropriate minimum expected TypeScript level is. The test:dist:types tests pass at least for now with TS 3.8 and later, but I'm not even sure if that's a sufficient indicator of compatibility, or how to really test for that. I am aware of typesVersions, but that doesn't seem like it really supports defining a minimum TS version.

So I think the "official" line here will be to not explicitly promise any explicit level of TS version compatibility for yaml, but to reconsider things when the CI tests start failing due to some new type addition.

@eemeli eemeli closed this as completed May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants