[CLA Approved] feat: AMD -> ES6 #7029

LeoDog896 · 2023-09-01T16:09:36Z

Fixes #6975, finishing a step for switching to an alternative ES6-only bundler like Vite.

(This is currently drafted as not all tests pass - there was weird ES6/AMD conversion hacks going on in the codebase).

This isn't a full refactor to ES6 classes and ESMified code. I've converted a few to classes when the files were small or when necessary (e.g. with the main MCT class), but I've tried to avoid many refactors and only focus on AMD -> ES6 conversion. If this PR is accepted, I'll probably lead up with a second PR to finish cleanup of the rest of the folder.

LeoDog896 · 2023-09-01T17:44:37Z

Random tidbit but there seems to be some odd TS issue happening (there is a partial block by microsoft/TypeScript#55172), so I'll need to switch some functions into anonymous lambdas in the meantime.

deploysentinel · 2023-09-01T17:55:02Z

Current Playwright Test Results Summary

✅ 166 Passing - ⚠️ 3 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 12/27/2023 06:56:30pm UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: 9dfbdaf

Started: 12/27/2023 06:51:56pm UTC

⚠️ Flakes

📄 functional/plugins/tabs/tabs.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Tabs View Renders tabbed elements Retry 1 • Initial Attempt	15.79% (6) 6 / 38 runs failed over last 7 days	18.42% (7) 7 / 38 runs flaked over last 7 days

📄 performance/tabs.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Tabs View Renders tabbed elements nicely Retry 1 • Initial Attempt	6.90% (2) 2 / 29 runs failed over last 7 days	3.45% (1) 1 / 29 run flaked over last 7 days

📄 functional/planning/timelist.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Time List Create a Time List, add a single Plan to it and verify all the activities are displayed with no milliseconds Retry 1 • Initial Attempt	0% (0) 0 / 40 runs failed over last 7 days	67.50% (27) 27 / 40 runs flaked over last 7 days

View Detailed Build Results

scarily small - can see why this e2e coverage issue is high priority

codecov · 2023-09-01T17:55:23Z

Codecov Report

Attention: 580 lines in your changes are missing coverage. Please review.

Comparison is base (715a448) 56.02% compared to head (9dfbdaf) 55.25%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7029      +/-   ##
==========================================
- Coverage   56.02%   55.25%   -0.78%     
==========================================
  Files         658      656       -2     
  Lines       26361    26617     +256     
  Branches     2546     2546              
==========================================
- Hits        14769    14707      -62     
- Misses      10893    11207     +314     
- Partials      699      703       +4

Flag	Coverage Δ		*Carryforward flag
e2e-full	`44.99% <66.66%> (+3.20%)`	⬆️	Carriedforward from 3564a47
e2e-stable	`58.11% <24.64%> (-0.44%)`	⬇️
unit	`48.83% <67.69%> (-0.34%)`	⬇️

*This pull request uses carry forward flags. Click here to find out more.

Files	Coverage Δ
example/generator/GeneratorMetadataProvider.js	`100.00% <100.00%> (ø)`
example/generator/SinewaveLimitProvider.js	`100.00% <100.00%> (ø)`
openmct.js	`100.00% <100.00%> (ø)`
src/MCT.js	`96.96% <100.00%> (-0.36%)`	⬇️
...c/plugins/URLIndicatorPlugin/URLIndicatorPlugin.js	`100.00% <100.00%> (ø)`
src/plugins/displayLayout/DisplayLayoutType.js	`100.00% <100.00%> (ø)`
src/plugins/filters/plugin.js	`100.00% <100.00%> (ø)`
src/plugins/flexibleLayout/plugin.js	`100.00% <100.00%> (ø)`
src/plugins/localTimeSystem/LocalTimeSystem.js	`100.00% <ø> (ø)`
src/plugins/localTimeSystem/plugin.js	`100.00% <100.00%> (ø)`
... and 64 more

... and 6 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 715a448...9dfbdaf. Read the comment docs.

LeoDog896 · 2023-09-01T19:32:59Z

Odd - that one failing playwright test seems to be very flaky. I ran it locally, and it failed the first time, and was successful the second time. Maybe it's a timing issue?

ozyx · 2023-09-01T20:28:47Z

Hi @LeoDog896!

Thanks for contributing to Open MCT! If you haven't already, please complete and submit a Contributor License Agreement (CLA). This is required before we can approve/merge your code.

Odd - that one failing playwright test seems to be very flaky. I ran it locally, and it failed the first time, and was successful the second time. Maybe it's a timing issue?

Yep, that test is a current known flake, so likely not due to these changes.

If this PR is accepted, I'll probably lead up with a second PR to finish cleanup of the rest of the folder.

I think that's a good idea. This will take a while to review, and it's probably best to take care of large changes like this in manageable chunks. While we're waiting for your CLA to go through, please let us know if you have any questions or if we can assist in any way. Thanks again for this work!

ozyx · 2023-10-05T20:47:06Z

Hi @LeoDog896 ,

Thanks for your patience. We're still waiting to hear back about your CLA. I'll follow up with the folks in charge of that to see if we can get a status.

In the meantime, there have been quite a few changes since this PR was opened. Could you give a shot at resolving the resulting conflicts and let us know if you run into any issues?

LeoDog896 · 2023-10-06T11:36:32Z

Sure! There shouldn't bee too many issues with merging; I'll resolve it in a bit.

ozyx · 2023-12-19T17:30:26Z

@LeoDog896 At long last, I've received a follow-up about your CLA being approved! I'll give a shot at bringing this PR up to speed as it's been sitting here for a while. Really appreciate your patience on this one.

LeoDog896 · 2023-12-20T15:57:13Z

Alright, thanks! @ozyx, if you're not able to do it, then let me know - I completely forgot after saying my previous comment 😅

src/api/objects/object-utils.js

src/plugins/autoflow/AutoflowTabularPlugin.js

src/plugins/autoflow/AutoflowTabularView.js

LeoDog896 · 2023-12-22T14:04:11Z

I'm not sure about the object-utils security issue mentioned above being a security issue, but 2 new (non-security) issues were caught because of the use of ES6.

ozyx · 2023-12-22T17:23:51Z

I'm not sure about the object-utils security issue mentioned above being a security issue, but 2 new (non-security) issues were caught because of the use of ES6.

Yep, looks like the scanner is catching some more stuff now that we're using ES6. Awesome!

ozyx

Really nice work! I've left a few suggestions, but this looks great overall. The tests are passing too! One thing I'd like to test is whether or not this changes how openmct is used as a dependency. I suspect it should remain very much the same, but I'd like to verify that before we go ahead with this.

src/api/objects/object-utils.js

src/api/api.js

src/plugins/summaryWidget/src/views/SummaryWidgetViewProvider.js

src/plugins/plugins.js

src/ui/registries/ViewRegistry.js

src/ui/registries/ToolbarRegistry.js

src/MCT.js

ozyx · 2023-12-22T23:50:56Z

Really nice work! I've left a few suggestions, but this looks great overall. The tests are passing too! One thing I'd like to test is whether or not this changes how openmct is used as a dependency. I suspect it should remain very much the same, but I'd like to verify that before we go ahead with this.

Testing against our testing environment with openmct as a dependency and have had no issues at all. Fantastic

ozyx

Couple additional suggestions

src/MCT.js

Co-authored-by: Jesse Mazzella <[email protected]>

ozyx

This is looking great-- super excited to get this merged. Just one minor configuration fix

LeoDog896 · 2023-12-27T18:56:09Z

This is looking great-- super excited to get this merged. Just one minor configuration fix

👍, anything else?

ozyx

LGTM

* refactor: fix issues arose from #7029 * refactor: move replaceAll -> replace in makeKeyString * fix: move order of escape to backslash first * style: lint

LeoDog896 added 5 commits September 1, 2023 11:35

feat: full amd -> es6 conversion

df7affc

fix: move MCT to ES6 class

dbb451f

fix: default drop, correct imports

fc1297b

fix: correct all imports

381c263

fix: property typo

bca638a

fix: avoid anonymous functions

084d0d9

LeoDog896 marked this pull request as ready for review September 1, 2023 17:51

fix: correct typo

912b511

scarily small - can see why this e2e coverage issue is high priority

fix: use proper uuid format

c85c644

ozyx changed the title ~~feat: AMD -> ES6~~ [CLA Needed] feat: AMD -> ES6 Sep 1, 2023

ozyx added the source:community Community contribution or request label Sep 1, 2023

LeoDog896 added 2 commits September 1, 2023 19:44

Merge branch 'master' into master

39466e7

Merge branch 'master' into master

a5beda1

ozyx changed the title ~~[CLA Needed] feat: AMD -> ES6~~ [CLA Pending] feat: AMD -> ES6 Oct 9, 2023

ozyx changed the title ~~[CLA Pending] feat: AMD -> ES6~~ [CLA Approved] feat: AMD -> ES6 Dec 19, 2023

LeoDog896 added 2 commits December 21, 2023 15:31

Merge remote-tracking branch 'upstream/master'

3dad3f4

style: fmt

000cdac

ozyx added type:enhancement type:maintenance tests, chores, or project maintenance and removed type:enhancement labels Dec 21, 2023

fix: import vue correctly, get correct layout

cad0288

github-advanced-security bot found potential problems Dec 22, 2023

View reviewed changes

src/api/objects/object-utils.js Dismissed Show resolved Hide resolved

src/plugins/autoflow/AutoflowTabularPlugin.js Show resolved Hide resolved

src/plugins/autoflow/AutoflowTabularView.js Show resolved Hide resolved

ozyx suggested changes Dec 22, 2023

View reviewed changes

ozyx added the pr:e2e:couchdb npm run test:e2e:couchdb label Dec 22, 2023

ozyx suggested changes Dec 23, 2023

View reviewed changes

src/MCT.js Outdated Show resolved Hide resolved

src/MCT.js Outdated Show resolved Hide resolved

ozyx reviewed Dec 23, 2023

View reviewed changes

src/MCT.js Outdated Show resolved Hide resolved

LeoDog896 and others added 6 commits December 22, 2023 19:31

Update src/api/api.js

951c92b

Co-authored-by: Jesse Mazzella <[email protected]>

Update src/plugins/plugins.js

3e97dfc

Co-authored-by: Jesse Mazzella <[email protected]>

Update src/plugins/plugins.js

b141453

Co-authored-by: Jesse Mazzella <[email protected]>

fix: suggestions

dec34ed

fix: drop unnecessary this.annotation initialization

3d75f35

fix: move all initialization calls to constructor

3564a47

LeoDog896 requested a review from ozyx December 23, 2023 01:48

ozyx suggested changes Dec 27, 2023

View reviewed changes

refactor: move vue dist import to webpack alias

9dfbdaf

ozyx added pr:e2e:couchdb npm run test:e2e:couchdb and removed pr:e2e:couchdb npm run test:e2e:couchdb labels Dec 27, 2023

ozyx self-requested a review December 27, 2023 20:12

ozyx approved these changes Dec 27, 2023

View reviewed changes

ozyx merged commit 2e03bc3 into nasa:master Dec 27, 2023
20 of 26 checks passed

LeoDog896 added a commit to LeoDog896/openmct that referenced this pull request Dec 28, 2023

refactor: fix issues arose from nasa#7029

2906e6f

LeoDog896 mentioned this pull request Dec 28, 2023

fix: cleanup from #7029 #7328

Merged

15 tasks

ozyx pushed a commit that referenced this pull request Dec 28, 2023

fix: cleanup from #7029 (#7328)

68e60e3

* refactor: fix issues arose from #7029 * refactor: move replaceAll -> replace in makeKeyString * fix: move order of escape to backslash first * style: lint

unlikelyzero modified the milestones: Target:3.3.0, Target:4.0.0 Feb 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CLA Approved] feat: AMD -> ES6 #7029

[CLA Approved] feat: AMD -> ES6 #7029

LeoDog896 commented Sep 1, 2023 •

edited

Loading

LeoDog896 commented Sep 1, 2023 •

edited

Loading

deploysentinel bot commented Sep 1, 2023 •

edited

Loading

Test Case Results

Test Case Results

Test Case Results

codecov bot commented Sep 1, 2023 •

edited

Loading

LeoDog896 commented Sep 1, 2023

ozyx commented Sep 1, 2023 •

edited

Loading

ozyx commented Oct 5, 2023

LeoDog896 commented Oct 6, 2023 •

edited

Loading

ozyx commented Dec 19, 2023

LeoDog896 commented Dec 20, 2023

LeoDog896 commented Dec 22, 2023

ozyx commented Dec 22, 2023

ozyx left a comment

ozyx commented Dec 22, 2023

ozyx left a comment

ozyx left a comment

LeoDog896 commented Dec 27, 2023

ozyx left a comment

[CLA Approved] feat: AMD -> ES6 #7029

[CLA Approved] feat: AMD -> ES6 #7029

Conversation

LeoDog896 commented Sep 1, 2023 • edited Loading

LeoDog896 commented Sep 1, 2023 • edited Loading

deploysentinel bot commented Sep 1, 2023 • edited Loading

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Test Case Results

Test Case Results

codecov bot commented Sep 1, 2023 • edited Loading

Codecov Report

LeoDog896 commented Sep 1, 2023

ozyx commented Sep 1, 2023 • edited Loading

ozyx commented Oct 5, 2023

LeoDog896 commented Oct 6, 2023 • edited Loading

ozyx commented Dec 19, 2023

LeoDog896 commented Dec 20, 2023

LeoDog896 commented Dec 22, 2023

ozyx commented Dec 22, 2023

ozyx left a comment

Choose a reason for hiding this comment

ozyx commented Dec 22, 2023

ozyx left a comment

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

LeoDog896 commented Dec 27, 2023

ozyx left a comment

Choose a reason for hiding this comment

LeoDog896 commented Sep 1, 2023 •

edited

Loading

LeoDog896 commented Sep 1, 2023 •

edited

Loading

deploysentinel bot commented Sep 1, 2023 •

edited

Loading

codecov bot commented Sep 1, 2023 •

edited

Loading

ozyx commented Sep 1, 2023 •

edited

Loading

LeoDog896 commented Oct 6, 2023 •

edited

Loading