search

theia-ide / yangster

Yangster is a YANG IDE based on Theia
Apache License 2.0
42 stars 17 forks source link

Yangster "next" version does not build #69

Closed marcdumais-work closed 5 years ago

marcdumais-work commented 5 years ago

There will be a release of Theia tomorrow, where something close to the current "next" version will become "latest", presumably breaking also "latest" of yangster.

$> bash theia-version.sh next
$> yarn
[...]
$ tsc && yarn lint
src/frontend/language/yang-language-client-contribution.ts(65,67): error TS2345: Argument of type '(uri: URI) => void' is not assignable to parameter of type '(e: URI) => any'.
  Types of parameters 'uri' and 'e' are incompatible.
    Type 'import("/home/marc/dev/yangster/yangster/node_modules/theia-sprotty/node_modules/@theia/core/lib/...' is not assignable to type 'import("/home/marc/dev/yangster/yangster/node_modules/@theia/core/lib/common/uri").default'.
      Types have separate declarations of a private property 'codeUri'.
src/frontend/yangdiagram/yang-diagram-manager.ts(30,60): error TS2345: Argument of type 'import("/home/marc/dev/yangster/yangster/node_modules/@theia/languages/lib/browser/language-clien...' is not assignable to parameter of type 'import("/home/marc/dev/yangster/yangster/node_modules/theia-sprotty/node_modules/@theia/languages...'.
  Types of property 'waitForActivation' are incompatible.
    Type '(app: import("/home/marc/dev/yangster/yangster/node_modules/@theia/core/lib/browser/frontend-appl...' is not assignable to type '(app: import("/home/marc/dev/yangster/yangster/node_modules/theia-sprotty/node_modules/@theia/cor...'.
      Types of parameters 'app' and 'app' are incompatible.
        Type 'import("/home/marc/dev/yangster/yangster/node_modules/theia-sprotty/node_modules/@theia/core/lib/...' is not assignable to type 'import("/home/marc/dev/yangster/yangster/node_modules/@theia/core/lib/browser/frontend-applicatio...'.
          Property 'measure' is missing in type 'FrontendApplication'.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
marcdumais-work commented 5 years ago

When building we get a mix of "next" and "latest" theia extensions. It looks-like we end-up pulling theia-sprotty from npm instead of using the local one, that correctly has @theia "next" dependencies.

from yangster/node_modules/theia-sprotty/package.json:

    "@theia/core": "latest",
    "@theia/editor": "latest",
    "@theia/filesystem": "latest",
    "@theia/languages": "latest",
    "@theia/monaco": "latest",

from yangster/theia-sprotty/package.json :

    "@theia/core": "next",
    "@theia/editor": "next",
    "@theia/filesystem": "next",
    "@theia/languages": "next",
    "@theia/monaco": "next",
marcdumais-work commented 5 years ago

Related to this recent commit I think: https://github.com/theia-ide/yangster/commit/6ee02d79d24b5326f83ba28ba373107b8fe91065

marcdumais-work commented 5 years ago

Confirmed: if I roll-back the last couple of commits, then "next" build correctly. Shall we revert to 90d9d8c2ef3a6d291e59cf95499157153b5c02fa and publish a new version?

JanKoehnlein commented 5 years ago

The problem is that theia-sprotty:0.1.20 depends on theia:latest. So for next builds, we cannot use 0.1.20 but have to use theia-sprotty:next. I think, we have to adapt the theia-version.sh script to also change this. We should convert it to JS anyway, such that it runs on all OSs.

Strangely, everything compiled well on my local repo, and I could as well do a latest release and a new next build. I had to checkout a fresh clone to reproduce it.

As the repo's master should always refer to next, I'll change that. The current latest build should be OK. Would be nice if you checked that.

JanKoehnlein commented 5 years ago

BTW, sprotty has moved to Eclipse, and we should refactor yangster to use the eclipse versions of sprotty and sprotty-theia.

JanKoehnlein commented 5 years ago

It's a mess. I tried for several hours now, but I cannot get the next build running, because theia-sprotty@next is completely messed up on npm. It's not worth fixing as the Eclipse package will have a different name anyway.

I propose to leave it as is and only produce latest releases until we port yangster to Eclipse sprotty.

marcdumais-work commented 5 years ago

Thanks for giving this a good try, @JanKoehnlein .

I propose to leave it as is and only produce latest releases until we port yangster to Eclipse sprotty.

+1

marcdumais-work commented 5 years ago

the Eclipse package will have a different name anyway

Will you rename the package by choice or is it mandated by the Foundation? I was wondering if we will have to rename the @theia packages of the main repo, once we move in the Foundation.

svenefftinge commented 5 years ago

Hi Marc, we will not have to rename the packages. That would be a big mess.

svenefftinge commented 5 years ago

btw. I fixed the master build of this repo, today. So it works again. But I think we should keep this ticket open, as the big work (migrating everything to the latest sprotty) is still todo.

svenefftinge commented 5 years ago

Oh, I see you created a separate ticket. So we can close this one.

marcdumais-work commented 5 years ago

I fixed the master build of this repo, today. So it works again

Thanks!