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

Conflicting declarations in theia.d.ts and theia.proposed.d.ts #11556

Closed
colin-grant-work opened this issue Aug 10, 2022 · 6 comments
Closed
Labels
plug-in system issues related to the plug-in system quality issues related to code and application quality

Comments

@colin-grant-work
Copy link
Contributor

colin-grant-work commented Aug 10, 2022

Bug Description:

Steps to Reproduce:

  1. In packages/plugin/tsconfig.json set compilerOptions.skipLibCheck: false.
  2. Open theia.d.ts
  3. Find AuthenticationProviderAuthenticationSessionsChangeEvent and observe that TS now points out that the interface is declared with different types in theia.proposed.d.ts

Additional Information

Various other problems are found with skipLibCheck: false, and straightforward fixes result in build errors in the code.

@colin-grant-work colin-grant-work added quality issues related to code and application quality plug-in system issues related to the plug-in system labels Aug 10, 2022
@colin-grant-work
Copy link
Contributor Author

@martin-fleck-at, since it looks like you added the authentication API to theia.d.ts, would you be willing to clean up theia.proposed.d.ts where to two conflict / where the latter is out of date with VSCode's proposals?

@martin-fleck-at
Copy link
Contributor

@colin-grant-work Sure, I can have a look!

martin-fleck-at added a commit to eclipsesource/theia that referenced this issue Aug 11, 2022
- Remove very out-dated backwards compatibility to proposed VS Code API
-- Get rid of provider id-based API
-- Remove deprecated 'hasSession' from API
-- Deprecate login/logout (now: createSession/removeSession) in Theia

Previously we were compatible with a very specific commit of the
proposed authentication API of VS Code 1.53.2. The API was moved from
its proposed state to stable with 1.54.0 to which we are fully
compatible.

Relates to eclipse-theia#11556
@martin-fleck-at
Copy link
Contributor

@colin-grant-work I had a look at the API and I do not think there is a good strategy to get the types proposed API and the stable API compatible. I therefore suggest to get rid of the proposed API since we only were compatible with a very specific commit of the proposed API in VS Code 1.53.2 anyway and therefore never fully compatible with 1.53.2 (see #10709 (comment) and the PR). Our current stable API matches the stable API from VS Code 1.54.0 and higher which I hope will be enough, what do you think?

@colin-grant-work
Copy link
Contributor Author

@martin-fleck-at, I agree. I don't believe VSCode does, and I don't believe we can or should guarantee ongoing compatibility with outdated proposal stages, so if the API has moved on, we should as well.

@martin-fleck-at
Copy link
Contributor

@colin-grant-work Perfect, I already implemented this in a PR here: #11564 ;-)

martin-fleck-at added a commit that referenced this issue Aug 12, 2022
- Remove very out-dated backwards compatibility to proposed VS Code API
-- Get rid of provider id-based API
-- Remove deprecated 'hasSession' from API
-- Deprecate login/logout (now: createSession/removeSession) in Theia

Previously we were compatible with a very specific commit of the
proposed authentication API of VS Code 1.53.2. The API was moved from
its proposed state to stable with 1.54.0 to which we are fully
compatible.

Relates to #11556
martin-fleck-at added a commit that referenced this issue Aug 12, 2022
- Remove very out-dated backwards compatibility to proposed VS Code API
-- Get rid of provider id-based API
-- Remove deprecated 'hasSession' from API
-- Deprecate login/logout (now: createSession/removeSession) in Theia

Previously we were compatible with a very specific commit of the
proposed authentication API of VS Code 1.53.2. The API was moved from
its proposed state to stable with 1.54.0 to which we are fully
compatible.

Relates to #11556
@colin-grant-work
Copy link
Contributor Author

Closing this as complete via #11564 and #11684

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plug-in system issues related to the plug-in system quality issues related to code and application quality
Projects
None yet
Development

No branches or pull requests

2 participants