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.

Sign up for GitHub

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

Jump to bottom

Fixes to run dart and flutter vscode extensions #6044

Merged
merged 10 commits into from
Sep 4, 2019
Merged

Fixes to run dart and flutter vscode extensions #6044

merged 10 commits into from
Sep 4, 2019

Conversation

akosyakov
Copy link
Member

@akosyakov akosyakov commented Aug 26, 2019 •
edited
Loading

What it does

Various fixes to run dart and flutter vscode extensions (#3999):

How to test

Review checklist

Reminder for reviewers

@akosyakov akosyakov added vscode issues related to VSCode compatibility dart issues related to the dart language labels Aug 26, 2019
@akosyakov akosyakov marked this pull request as ready for review August 27, 2019 06:59
@akosyakov akosyakov requested a review from a team as a code owner August 27, 2019 06:59
@svenefftinge
Copy link
Contributor

I tried it with https://github.com/flutter/flutter_web (my fork of it) and ran into multiple issues:

  • it asked me to install/update packages and ran pub get and did so but seemed to hang at some point.
  • at the same time the IDE was flooded with disgnostics (because of missing packages), which slowed the frontend down.
  • there were multiple errors in the console, complaining about usage of a disposed connection

@svenefftinge
Copy link
Contributor

Interestingly, eventually everything seemed to be installed and working nicely.

@akosyakov akosyakov mentioned this pull request Aug 27, 2019
Closed
@DanTup
Copy link
Contributor

DanTup commented Aug 27, 2019

seemed to hang at some point.

As in the Dart process hanged, or something in the IDE? If it's the Dart process - it can be slow sometimes doing things like package resolution, but it shouldn't take forever. What's the simplest way for me to run this and see it?

at the same time the IDE was flooded with disgnostics (because of missing packages), which slowed the frontend down.

How many do you mean by flooded? Are you still opening the whole flutter_web project, or just the example project? If just the example project I wouldn't expect many errors since there shouldn't be much code referencing the packages.

there were multiple errors in the console, complaining about usage of a disposed connection

Are these issues you think in theia, or the extension? If you think the extension, do you still have the text?

Thanks!

@akosyakov
Copy link
Member Author

@DanTup thank for your input! I will investigate it next day. Probably, just Theia needs a bit of optimizations.

@akosyakov akosyakov force-pushed the ak/flutter_support branch 3 times, most recently from 316d17f to 4fab226 Compare September 2, 2019 09:45
This was referenced Sep 2, 2019
Closed
Closed
@akosyakov
Copy link
Member Author

akosyakov commented Sep 2, 2019 •
edited
Loading

@svenefftinge Could you try again? There were 2 clients doing expensive computations based on problem markers: problem tab decorator and problem status element. Now they are debouncing and computations are optimized.

@vince-fugnitto @fangnx Could you please test that problem tab bar decorations are still working properly? It does not need to be done with the Dart extension.

svenefftinge
svenefftinge previously approved these changes Sep 2, 2019
View reviewed changes
@svenefftinge svenefftinge dismissed their stale review September 2, 2019 14:32

diagnostics are strange

@svenefftinge
Copy link
Contributor

The diagnostics are updated weirdly.
I did the following:

  • open a file
  • make a syntactically invalid change (no errors)
  • save (still no errors)
  • close editor (errors in navigator)
  • reopen (errors)
  • fix syntax error (more errors on strange places (seems like the buffer in LS is corrupt)
  • copy all, clear editor and paste (no more errors)

@svenefftinge
Copy link
Contributor

Also I noticed that after a change in the editor I can no longer apply any of the code actions proposed on components.
I get

winjs.base.js:1230 Uncaught (in promise) TypeError: e.offsetAt is not a function
    at t.RefactorCommands.<anonymous> (/tmp/vscode-unpacked/dart-code-3.4.0-beta.3.vsix/extension/out/dist/extension.js:1)
    at Generator.next (<anonymous>)
    at /tmp/vscode-unpacked/dart-code-3.4.0-beta.3.vsix/extension/out/dist/extension.js:1
    at new Promise (<anonymous>)
    at r (/tmp/vscode-unpacked/dart-code-3.4.0-beta.3.vsix/extension/out/dist/extension.js:1)
    at t.RefactorCommands.getRefactor (/tmp/vscode-unpacked/dart-code-3.4.0-beta.3.vsix/extension/out/dist/extension.js:1)
    at t.RefactorCommands.<anonymous> (/tmp/vscode-unpacked/dart-code-3.4.0-beta.3.vsix/extension/out/dist/extension.js:1)
    at Generator.next (<anonymous>)
    at /tmp/vscode-unpacked/dart-code-3.4.0-beta.3.vsix/extension/out/dist/extension.js:1
    at new Promise (<anonymous>)

@DanTup
Copy link
Contributor

DanTup commented Sep 2, 2019

Let me know if it'd help much to have a build of the extension that isn't webpacked (we have to pack it to stop VS Code showing warnings due to slow sync module loading, but it probably makes understanding what's going on while debugging here harder).

offsetAt is a function on TextDocument, so ths error above is probably here:

https://github.com/Dart-Code/Dart-Code/blob/31340f4b9afedc7098bed7b10548aa4a25120dfc/src/extension/commands/refactor.ts#L68

@akosyakov
Copy link
Member Author

Just noticed that with this PR, the flutter outline works:
Screen Shot 2019-09-03 at 14 00 17

This was referenced Sep 3, 2019
Closed
Closed
@akosyakov akosyakov force-pushed the ak/flutter_support branch 2 times, most recently from 52a4f53 to fc03ce2 Compare September 3, 2019 12:07
@akosyakov akosyakov mentioned this pull request Sep 3, 2019
Open
@akosyakov
Copy link
Member Author

@svenefftinge You can try again, now document changes and command args should be good. There is still an issue to resolve for code actions: #6095 I will have a look tomorrow.

svenefftinge
svenefftinge approved these changes Sep 3, 2019
View reviewed changes
Copy link
Contributor

@svenefftinge svenefftinge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Works well for me now.

@akosyakov akosyakov mentioned this pull request Sep 4, 2019
Closed
@DanTup
Copy link
Contributor

DanTup commented Sep 4, 2019

As a heads up, I published a new Dart extension yesterday (v3.4.0) and it had a bug in running tests from Code Lens, so I also published a v3.4.1 with a fix this morning.

Release notes are here: https://dartcode.org/releases/v3-4/

I don't think there were any new APIs used in it, but I figured it worth calling out in case you see things failing that didn't before.

@akosyakov
[vscode] stub an extension kind as a local VS Code extension
230c2ed 
i.e. not running on a remote workspace

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[debug] ignore additional breakpoints returned by setBreakpoints re…
73cca79 
…quest

otherwise it fails `undefined` and swallows updates for other breakpoints

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] implement Plugin.isActive check
b71fad2 
before it was stub with `true` always

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] implement selection and visible tree view APIs
99e1273 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
fix #6039: implement vscode.env.openExternal API
cfda1b4 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] list onUri as a supported activation event
f34d20c 
It should be triggered when a os-wide URI is opened, for cloud case it is not necessary, so stubbing for now. As soon as os-wide opening of URIs is implemented for Electron this event should be fired as well.

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
fix #6083: optimize tab bar decorations rendering
67a04cb 
- cache decorations
- debounce rendering

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
fix #6084: optimize problem status rendering
737d348 
- debounce status update
- travers all markers only once, not for each Uri and then for each severity

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] fix #6091: pass proper document change rangeOffset
a2cc0ee 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
[plugin] fix #6092: make sure command arguments are passed safely via…
6b80981 
… jsonrpc

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov
Copy link
Member Author

I've tested changes with following VS Code extensions

  • java + java debug
  • git
  • sample tree views

Everything seems fine, i.e. code lenses and actions are there, git integration too, and tree view content is proper. Also tried with typescript extension, but it cannot function properly without exposing internal monaco commands.

I'm going to merge if the build is green and no objections.

@akosyakov
Copy link
Member Author

akosyakov commented Sep 4, 2019 •
edited
Loading

@DanTup I was testing always against the latest master state of Dart extension, so it should be fine.

Updated I've retested with 3.4.1 and it works nicely as well.

@akosyakov akosyakov merged commit 8523b99 into master Sep 4, 2019
@akosyakov akosyakov deleted the ak/flutter_support branch September 4, 2019 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@svenefftinge svenefftinge svenefftinge approved these changes

@AlexTugarev AlexTugarev Awaiting requested review from AlexTugarev

@fangnx fangnx Awaiting requested review from fangnx

@vince-fugnitto vince-fugnitto Awaiting requested review from vince-fugnitto

Assignees
No one assigned
Labels
dart issues related to the dart language vscode issues related to VSCode compatibility
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[vscode] tree view nodes are never cleaned up
3 participants
@akosyakov @svenefftinge @DanTup