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

Replace util.js polyfills with JS natives... #11871

Merged
merged 2 commits into from
Sep 26, 2023
Merged

Replace util.js polyfills with JS natives... #11871

merged 2 commits into from
Sep 26, 2023

Conversation

fredcw
Copy link
Contributor

@fredcw fredcw commented Sep 25, 2023

Replace all instances of js/misc/util.js polyfills (findIndex(), find(), each(), filter() & map()) with JS natives in cinnamon and delete these polyfills from util.js.

These were originally added for performance but are now slower in current cjs. They are no longer used by any xlets in linuxmint/cinnamon-spices-applets, linuxmint/cinnamon-spices-desklets, or linuxmint/cinnamon-spices-extensions. However, they are still used in the cinnamon menu applet so this PR depends on #11865 also being merged.

+grouped-window-list applet: Always add alert style to thumbnail when app window needs attention and implement same style removal when app is focused (not previously implemented)

+grouped-window-list applet: used const instead of let where possible.

Replace js/misc/util.js polyfills (findIndex(), find(), each(), filter() & map()) with JS natives in cinnamon.

+grouped-window-list applet: Always add alert style to thumbnail when app window needs attention and implement same style removal when app is focused (not previously implemented)

+grouped-window-list applet: used 'const' instead of 'let' where possible.
@mtwebster mtwebster merged commit e4b39d3 into linuxmint:master Sep 26, 2023
3 checks passed
@fredcw fredcw deleted the polys2 branch September 26, 2023 19:05
@brownsr
Copy link
Member

brownsr commented Sep 27, 2023

Applet fails to launch:

[grouped-window-list@cinnamon.org]: find is not a function
[grouped-window-list@cinnamon.org]: Failed to evaluate 'main' function on applet: grouped-window-list@cinnamon.org/16
Cjs-Message: 08:11:25.405: JS LOG: [LookingGlass/trace]
<----------------
_connect@/usr/share/cinnamon/js/misc/state.js:250:24
connect@/usr/share/cinnamon/js/misc/state.js:271:25
GroupedWindowListApplet@/usr/share/cinnamon/applets/grouped-window-list@cinnamon.org/applet.js:211:20
main@/usr/share/cinnamon/applets/grouped-window-list@cinnamon.org/applet.js:1022:12
createApplet@/usr/share/cinnamon/js/ui/appletManager.js:597:25
addAppletToPanels@/usr/share/cinnamon/js/ui/appletManager.js:372:34
finishExtensionLoad@/usr/share/cinnamon/js/ui/appletManager.js:98:14
_init/<@/usr/share/cinnamon/js/ui/extension.js:275:32
---------------->
Cjs-Message: 08:11:25.406: JS LOG: [LookingGlass/error]
[grouped-window-list@cinnamon.org]: Applet grouped-window-list@cinnamon.org: Could not create applet object.
[grouped-window-list@cinnamon.org]: Error importing applet.js from grouped-window-list@cinnamon.org
Cjs-Message: 08:11:25.406: JS LOG: [LookingGlass/trace]

@fredcw
Copy link
Contributor Author

fredcw commented Sep 27, 2023

@brownsr Thanks. Yes, I changed state.js and then forgot to add it to the commit 🤦‍♂️. I'll add it in a new PR

@fredcw
Copy link
Contributor Author

fredcw commented Oct 1, 2023

@mtwebster Unrelated to this PR but I was wondering: in GWL applet.js are the following comments:

line 205

// key-function pairs of actions that can be triggered from the store's callback queue. This allows the
// applet to avoid passing down the parent class down the constructor chain and creating circular references.

line 262

// Passing an empty object instead of `this` because its only used by SignalManager to bind the callback, which
// we already do here. Otherwise, it creates more circular references.

line 845

// TODO: Figure out exactly which properties on this applet constructor the Cinnamon APIs needs for all modes of
// DND, so we can kill the _delegate reference. Long term, a PR to Cinnamon should be opened fixing circular
// object reference structures for the applet and desklet classes.

Do you know of any reason why circular references should be avoided or perhaps this isn't necessary now?

Edit: btw, there seems to be an easy way to avoid circular references by passing a function that returns an object rather than passing the object itself:

i.e. instead of:

Class Foo {
   this.bar = new Bar(this);
}

Class Bar {
   constructor(foo) {
      this.foo = foo;
      this.foo.someFooFunction();

use:

Class Foo {
   this.bar = new Bar( () => this );
}

Class Bar {
   constructor(foo) {
      this.foo = foo;
      this.foo().someFooFunction();

@mtwebster
Copy link
Member

Seeing as the gwl applet is really the only place we worry about any of this (in Cinnamon proper, at least), I doubt this is as much of an issue as was being made here. Some googling also gives me the impression JS garbage collection is robust enough to handle it np.

We end up with circular refs all over the place now using _delegate in popup menus, xlets, etc... so even if it is an issue with GC it doesn't seem like a major one. We could always clear _delegates when we're cleaning up these things - that seems like it'd be easier than tying our hands ahead of time. That second snippet just seems like it'd be confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants