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

chore: Updated to Theia 1.31.1 #1662

Merged
merged 8 commits into from
Nov 29, 2022
Merged

chore: Updated to Theia 1.31.1 #1662

merged 8 commits into from
Nov 29, 2022

Conversation

kittaakos
Copy link
Contributor

@kittaakos kittaakos commented Nov 10, 2022

Motivation

Change description

Other information

Switched to @theia/*@1.31.1:

  • Aligned the p-queue version to Theia.
  • Updated to react@18+ from Theia. Replaced the ReactDOM#render calls with createRoot.
  • Added react-virtualized as a dependency. It was removed from Theia.
  • Updated to react-select@5.6.0.
  • Removed custom theme registration patch. Theia supports theme service customization and the default theme despatching on the OS theme.
  • Removed tab bar toolbar customization. IDE2 switched to the ellipses (...) icon from font-awesome to codicon. (Reverted)
tabbar-toolbar-customization.mp4
  • Removed the debug session manager customization. The customization solved the same problem as #994. Steps to verify:
    • Auto-save is on,
    • Create a new sketch,
    • Attach and select a debug-enabled board (e.g., Arduino Zero (Programming Port)),
    • Click Upload on the toolbar,
    • Click on the Debug toolbar item,
    • Debugger starts and hits the breakpoint if you have any, but IDE2 does not prompt Save as.
  • Removed the #856 patch when saving the settings. Saving the settings must work with it.
  • Removed the status bar customization. Theia does not update the status bar on every keydown event and will spare the CPU. Avoid updating the status bar update is crucial for the language server's performance. The change was here. Steps to verify (Advanced!):
    • Open the DevTools and search for status-bar-view-model.ts with Ctrl/Cmd+P. Put a breakpoint to line 133.
    • Start typing in the opened sketch editor,
    • The debugger does not hit the breakpoint, and the didChange event does not fire.
    • Press Ctrl/Cmd+K in the editor. monaco's key chords functionality activates.
      Screen Shot 2022-11-11 at 14 14 54
    • Now, press any key, and the debugger hits the breakpoint. This verifies that any arbitrary key press won't slow IDE's UI as the status bar update is not happening.
  • Removed widget manager customization to avoid duplicate editor tabs. It was fixed upstream: eclipse-theia/theia#11450.
  • Switched to Theia's new window title service API. Closes #1656.
  • Removed workspace service customization. It's part of Theia via eclipse-theia/theia#11571. Step to verify: open another sketch from the Sketchbook via the context menu > Open Sketch in New Window. If it works, you have helped verify that IDE2 windows can still pass arguments between each other without tricks in the IDE2 code-base.
  • Removed the patch of eclipse-theia/theia#11689. IDE2 must pick up keyboard layout changes without stopping/starting.
  • Removed the customization of eclipse-theia/theia#11229. It was fixed in Theia.
  • Adapted the menu factory APIs to Theia. Steps to verify: if the visibility and enablement of the File > New Remote Sketch menu item is correct, and you can see the accelerator next to it, it's working as expected.
  • Added a patch for eclipse-theia/theia#11600. Steps to verify:
    • Start IDE2,
    • Select Reload Window from the Command Palette,
    • Create a new sketch,
    • It works.
  • Added a patch for eclipse-theia/theia#11853.
  • Revert patch for #1261. The fix is available from Theia. IDE2 must keep the selected language between sessions.
  • Disabled preference schema validation in the bundled to speed up startup: eclipse-theia/theia#11189.
  • Set editor.bracketPairColorization.enabled to false by default.
  • Removed all @theia/typehierarchy UI features from IDE2, (eclipse-theia/theia@16c88a5)
  • [dev]: Removed webpack.config.js from Git. They're not required anymore.
  • [dev]: Removed depcheck plugin from packager. depcheck unreliably marked npm packages as unused and filtered them out. It's because Theia uses dynamic require calls, and tools (such as depcheck) cannot detect whether a module is needed at runtime.
  • [dev]: Improved packager's dev experience when bundling fails.

Closes #1655
Closes #1656

Reviewer checklist

  • PR addresses a single concern.
  • The PR has no duplicates (please search among the Pull Requests before creating one)
  • PR title and description are properly filled.
  • Docs have been added / updated (for bug fixes / features)

@kittaakos kittaakos added type: enhancement Proposed improvement topic: infrastructure Related to project infrastructure topic: code Related to content of the project itself labels Nov 10, 2022
@kittaakos kittaakos force-pushed the theia-1.31.1 branch 3 times, most recently from e9fe466 to 2bf972c Compare November 10, 2022 16:23
@per1234 per1234 added the topic: theia Related to the Theia IDE framework label Nov 11, 2022
@kittaakos kittaakos marked this pull request as ready for review November 11, 2022 14:33
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

UPDATE: Fixed by 3b8ed7f

Parts of the IDE update dialog are missing

To reproduce

  1. Start Arduino IDE.
  2. Wait for the 2.0.2 update notification dialog to appear.

🐛 The introductory sentence and changelog are missing from the dialog:

image

Expected behavior

image

Arduino IDE version

b5afe99 (tester build for 5e8a1f7)

Operating system

Windows

Operating system version

10

Additional context

The issue does not occur with any of the builds from the main branch (e.g., 2.0.2 with update notification triggered by changing to the "nightly" channel).

Akos Kitta added 3 commits November 22, 2022 15:19
Closes #1655

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@kittaakos
Copy link
Contributor Author

Parts of the IDE update dialog are missing

It should be working.


As requested, I removed all UI of @theia/typehierarchy. It was added to Theia with eclipse-theia/theia@16c88a5. If IDE2 wants to run VS Code extensions, it must deal with the contributed type-hierarchy features. In the long run, if feasible, IDE2 should enable clangd's type-hierarchy feature.

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

UPDATE: fixed by f338106

The **WiFi101 / WiFiNINA Firmware Updater** dialog is empty the second time you open it.

To reproduce

  1. Select Tools > WiFi101 / WiFiNINA Firmware Updater from the Arduino IDE menus.
    🙂 The dialog is populated as expected:
    image
  2. Click the X icon in the dialog.
  3. Select Tools > WiFi101 / WiFiNINA Firmware Updater from the Arduino IDE menus.

🐛 The dialog is empty:

image

Expected behavior

"WiFi101 / WiFiNINA Firmware Updater" dialog is populated any time it is opened.

Arduino IDE version

0d9b47b (tester build for 9e042ae)

Operating system

Windows

Operating system version

10

Additional context

I see an error in the logs the second time I open the dialog:

2022-11-23T07:36:47.218Z root ERROR Error: Minified React error #409; visit https://reactjs.org/docs/error-decoder.html?invariant=409 for the full message or use the non-minified dev environment for full errors and additional helpful warnings.
    at Yc.Qc.render.Yc.render (file:///C:/ide%202/rev/3-0d9b47b/resources/app/lib/bundle.js:2:9548299)
    at b.onUpdateRequest (file:///C:/ide%202/rev/3-0d9b47b/resources/app/lib/bundle.js:2:2237015)
    at b.e.processMessage (file:///C:/ide%202/rev/3-0d9b47b/resources/app/lib/bundle.js:2:10434587)
    at h (file:///C:/ide%202/rev/3-0d9b47b/resources/app/lib/bundle.js:2:1457698)
    at t (file:///C:/ide%202/rev/3-0d9b47b/resources/app/lib/bundle.js:2:1456674)
    at p (file:///C:/ide%202/rev/3-0d9b47b/resources/app/lib/bundle.js:2:1457869)

The issue does not occur with the build from the main branch.

 - removed the firmware updater dialog widget
 - let the patched react dialog to render the content

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@kittaakos
Copy link
Contributor Author

There is another regression when creating a remote sketch. The spinner CSS selector has been removed from Theia. They kept the icon, but the --theia-icon-loading is gone. We have to backport it.

missing-spinner.mp4

 - it has been removed from Theia
 - aligned the DOM structure to previous the version when using widget

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@kittaakos
Copy link
Contributor Author

Expected behavior

"WiFi101 / WiFiNINA Firmware Updater" dialog is populated any time it is opened.

It should work now. Thank you for finding it.

There is another regression when creating a remote sketch

This is also fixed.

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

UPDATE: fixed by ba16dcf

The **Configure and Upload** dialog is empty the second time you open it.

To reproduce

Equipment

  • Any board that is configured for "user provided fields".

    For example, the boards that can produce a network port:
    • MKR1000
    • Any ESP8266-based board
    • Any ESP32-based board
    • MKR WiFi 1010 (advanced configuration is required)
    • Nano 33 IoT (advanced configuration is required)

Steps

  1. Connect and set up the board that is configured for "user provided fields".
  2. Select the board and port in the Arduino IDE.
  3. Select Sketch > Configure and Upload from the Arduino IDE menus.
    🙂 The dialog is populated as expected:
    image
  4. Click the X icon in the dialog.
  5. Select Sketch > Configure and Upload from the Arduino IDE menus.

🐛 The dialog is empty:

image

Expected behavior

"Configure and Upload" dialog is populated any time it is opened.

Arduino IDE version

f8b97fc (tester build for bfcb518)

Operating system

Windows

Operating system version

10

Additional context

I see an error in the logs the second time I open the dialog:

2022-11-28T04:58:55.930Z root ERROR Error: Minified React error #409; visit https://reactjs.org/docs/error-decoder.html?invariant=409 for the full message or use the non-minified dev environment for full errors and additional helpful warnings.
    at qc.Qc.render.qc.render (file:///C:/ide%202/rev/4-f8b97fc/resources/app/lib/bundle.js:2:9548586)
    at h.onUpdateRequest (file:///C:/ide%202/rev/4-f8b97fc/resources/app/lib/bundle.js:2:2237982)
    at h.e.processMessage (file:///C:/ide%202/rev/4-f8b97fc/resources/app/lib/bundle.js:2:10434874)
    at h (file:///C:/ide%202/rev/4-f8b97fc/resources/app/lib/bundle.js:2:1458665)
    at t (file:///C:/ide%202/rev/4-f8b97fc/resources/app/lib/bundle.js:2:1457641)
    at p (file:///C:/ide%202/rev/4-f8b97fc/resources/app/lib/bundle.js:2:1458836)

The issue does not occur with the build from the main branch.

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@kittaakos
Copy link
Contributor Author

Expected behavior

"Configure and Upload" dialog is populated any time it is opened.

It should work now. Please take another look. Thank you!

@@ -0,0 +1,26 @@
#!/usr/bin/env node
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice one @kittaakos ❤️
What if we run this script in a pre-commit with husky?

Also, I would probably rename it with .js extension. Or is there a reason why you did otherwise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if we run this script in a pre-commit with husky?

Good idea. Feel free to enhance it once it's on the main branch.

Also, I would probably rename it with .js extension. Or is there a reason why you did otherwise?

Thanks! As mentioned in the usage, I prefer to run it as an executable from a shell. This is how the bin hoisting works with yarn on POSIX. Check the files under ./node_modules/.bin as an example. Of course, it will fail from CMD.exe, but you can run it as node scripts\\sort-dependencies package.json. Or if you have a shell on Windows or using GitBash, ./scripts/sort-dependencies ./arduino-ide-extension/package.json works. Why do you think we should have the .js extension?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you think we should have the .js extension?

I suggested it to be consistent with other scripts we already have in the scripts folder, but I see your point. Maybe it would make sense to change the others into shell executable scripts (I'm not suggesting to do it in this PR).

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@AlbyIanna
Copy link
Contributor

AlbyIanna commented Nov 29, 2022

UPDATE:
Will be solved in: #1720

This contextual menu icon in the sketch control toolbar seems a little off to me:

Screenshot 2022-11-28 at 14 58 11

I found that adding this CSS rule would fix it:

#arduino-open-sketch-control--toolbar {
    text-align: center;
}

Copy link
Contributor

@AlbyIanna AlbyIanna left a comment

Choose a reason for hiding this comment

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

Code is fine to me ✅

Thank you, Akos!

@per1234 per1234 linked an issue Nov 29, 2022 that may be closed by this pull request
3 tasks
@kittaakos
Copy link
Contributor Author

This contextual menu icon in the sketch control toolbar seems a little off to me:

Follow-up: #1720

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

Everything is working perfectly for me now on Windows and Linux.


I did a survey of the existing issues labeled "topic: theia"

PR resolves:

Partially resolves:

Not resolved (but not expected to):


Thanks Akos!

@kittaakos
Copy link
Contributor Author

  • breadcrumbs.enabled has no effect

The breadcrumbs feature has been forcefully removed from IDE2.

  • window.titleBarStyle has no effect

This has also been removed long ago.

  • workbench.editor.highlightModifiedTabs has no effect

I do not know what's this.

I can create a follow-up with these items.

@per1234
Copy link
Contributor

per1234 commented Dec 1, 2022

Hi @kittaakos

breadcrumbs.enabled has no effect

The breadcrumbs feature has been forcefully removed from IDE2.

window.titleBarStyle has no effect

This has also been removed long ago.

If the settings are not expected to work, I think it is reasonable to explain that in #1077 and consider documenting the fact in the issue report to be sufficient. These settings are intended to be targeted exclusively to advanced users so we don't need to be so concerned about the user experience being polished.

I don't know whether any further action to improve on the current situation is possible.

workbench.editor.highlightModifiedTabs has no effect

I do not know what's this.

You can see the feature in effect in VS Code here (I am using the "Light+ (default light)" theme in the screenshot, but I also see the border in all the other themes I checked: "Dark (Visual Studio)", "Light (Visual Studio)", and "Dark High Contrast"):

image

@kittaakos
Copy link
Contributor Author

I can create a follow-up with these items.

#1733

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself topic: infrastructure Related to project infrastructure topic: theia Related to the Theia IDE framework type: enhancement Proposed improvement
Projects
None yet
3 participants