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

Typescript CanvasWidget compilation error #559

Open
codie3611 opened this issue Mar 5, 2020 · 5 comments
Open

Typescript CanvasWidget compilation error #559

codie3611 opened this issue Mar 5, 2020 · 5 comments

Comments

@codie3611
Copy link

I added the project with yarn and then get the following error message from ts:

Failed to compile.

node_modules/@projectstorm/react-canvas-core/dist/@types/src/CanvasEngine.d.ts
(12,5): Property 'canvasReady' of type '(() => void) | undefined' is not assignable to string index type '(event: BaseEvent) => any'.

I'm using typescript 3.7.5. Anyone a clue how to get around this?

@codie3611
Copy link
Author

In case anyone wonders, editing tsconfig to contain "strictNullChecks": false worked partially. Now I get:

Failed to compile.

/home/lasso/programming/ts/lasso-hyperspace-client/node_modules/@projectstorm/react-diagrams-core/dist/@types/src/DiagramEngine.d.ts
(24,147): Cannot find module '../../react-canvas-core/dist/@types'.

@codie3611
Copy link
Author

There seems to be relative imports, which have a wrong level (2 times up, instead of 4 times up). Can hotfix this, but I prefer another more stable solution 😒

@timmartin
Copy link

I'm seeing this second problem ( Cannot find module '../../react-canvas-core/dist/@types') myself. It's preventing me from properly type checking any code dependent on this library, which is a bit of a shame.

I've done some digging, and here's what I think is happening (some of this is speculative, I'm not really a Typescript expert):

  • The type definition files are deployed to a place one directory deeper than they are while they are being compiled (they output to @types)
  • Ordinarily this wouldn't matter at all, because all the imports are absolute so it doesn't matter where the files are.
  • However, in some cases there's an anonymous type (or a derived type that isn't imported?) that Typescript needs to refer to in the output that is only implicitly in the input, so typescript has to refer to this as import('xyz').Whatever in the output code.
  • Normally this will be fine, because the inline import() will use an absolute import, the same as the imports at the top of the file, and those work just fine.
  • However, in the case of react-diagrams, the sibling packages are being built using lerna, which uses symlinks to make the sibling packages within the repo appear to be in the local node_modules even though they're elsewhere in the repo.
  • So during the build Typescript looks up the absolute import of e.g. @projectstorm/react-diagrams-core and follows the symlink to resolve this to a relative path it needs to load the actual file from
  • When Typescript needs to get hold of the filename in order to insert it into the import(...) statement in the output expression, it uses the actual path of the file it's obtained rather than the original absolute import. This resultant path is valid in the scope it's being built in, but not in the place it's eventually deployed.

I'm not 100% sure this is what's happening; it doesn't make any sense to me that it would matter whether the file is accessed via a symlink or not. I did some experiments and it seemed like that was how it was behaving, but I never quite reproduced exactly the same scenario, so I might have just been hitting some other edge case in Typescript. There are some issues on the Typescript repo (e.g. this one) that look like they might be related.

I experimented with manually patching my node_modules directory to make all the inline import() expressions use an absolute path, and it worked fine. However, this doesn't help with how we force Typescript to ouput the absolute path.

The good news is that it looks to me like the problem might just go away if react-diagrams is compiled using Typescript 3.9 - I compiled the library with 3.9 and it looked OK to me, though I haven't managed to try test this in integration with my real code.

Is it simple enough to upgrade to Typescript 3.9 in this repo, or do we risk breaking backward compatibility for library users?

@timmartin
Copy link

Sadly my comment about Typescript 3.9 fixing this was a complete red herring, I was comparing the wrong file. It turns out Typescript 3.9 doesn't fix this at all.

timmartin added a commit to timmartin/python-code-analyzer that referenced this issue Jun 4, 2020
There's a bug in react-diagrams:

projectstorm/react-diagrams#559

that causes an unavoidable Typescript error in all libraries that
build it. I don't see a good solution without making changes within
Typescript, so for the time being I'm going to suppress this.
@timmartin
Copy link

This problem seems to have been resolved for me as of the v6.4.2 release. Can anyone else confirm?

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

No branches or pull requests

3 participants