Review of "GNOME Widget Center" version 3

Details Page Preview

GNOME Widget Center is a simple and flexible way to add useful widgets to your GNOME desktop.It lets you easily manage, arrange, and customize different widgets in one place.You can add tools such as clocks, system information, media controls, shortcuts, and more.Each widget is designed to be lightweight, clean, and easy to configure. Make your desktop more useful and personal without changing the way GNOME works.

Extension Homepage
https://github.com/xenlism/gnome-widget-center

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

Shexli (experimental) warning 2

Shexli found 2 issues that may need reviewer attention.

EGO-X-004 warning

extensions should avoid synchronous file IO in shell code

Shell code should avoid synchronous file IO APIs like `GLib.file_get_contents()` and `Gio.File.load_contents()`.

File Operations

  • lib/fsUtils.js:59
    file.load_contents(null)
  • lib/fsUtils.js:73
    file.load_contents(null)

EGO-P-007 warning

JavaScript files should be reachable from extension.js or prefs.js

Some JavaScript files are not reachable from `extension.js` or `prefs.js` imports.

Don't include unnecessary files

  • i18n/de.js
  • i18n/en.js
  • i18n/es.js
  • i18n/ja.js
  • i18n/th.js
  • i18n/zh.js
  • lib/architectWidgetKit.js
  • lib/calendarGridKit.js
  • lib/iconAccentColor.js
  • lib/mediaApi.js
  • lib/shell/halfCircleGaugeKit.js
  • lib/shell/widgetTooltip.js
  • lib/systemCalendarEvents.js
  • lib/systemMetricsApi.js
  • lib/utils.js
  • lib/widgetConfigDefaults.js
  • widget-center-prefs-app.js
  • widgets/calendar-big/widget.js
  • widgets/calendar-events/widget.js
  • widgets/calendar-header/i18n/de.js

All Versions

Version Status
4 Unreviewed
3 Rejected
2 Rejected
1 Rejected

Previous Reviews on this Version

xenatt posted a review
**EOG-X-004 (sync file IO)** Yeah, these are real sync calls, no argument there. But I don't think they're really a live blocking concern for the shell process in practice. Line 59 (`readTextFile`) is called from shell code — `widgetConfigReader.js`, `storageService.js`, `themeService.js`, and also as a constructor fallback in a couple widgets (`xtile`, `geek-architect`). But all of these are very small local JSON files like `config.json`, `layout.json`, `metadata.json`, and they are only read once when the widget is constructed. So the actual blocking window is basically tiny, almost nothing in practice. Also most of these call sites already have async siblings available (`readTextFileAsync`, etc.) and those are used everywhere else in the codebase. The sync path is mainly there for the few places where it's inside a JS constructor and can't `await` — อันนี้เป็นข้อจำกัดของ flow มากกว่า. For Line 73 (`readBytesFile`), this one is actually not called anywhere in the codebase right now, sync or otherwise. It's kept intentionally though. There's a note about this in our internal design doc. Basically it's a fallback utility following the same pattern as `readTextFile`, so if some widget constructor in the future needs to do a synchronous binary read, we already have the utility ready. Since there is no live call site for it at all, it can't really be blocking anything today. So overall, I think the finding is valid from the pattern-matching perspective, but the actual real-world impact is very low / basically none. One is a bounded-size local file read that happens once, and the other one isn't called at all. Happy to add an inline comment on both places to explain the reason, if that helps future reviewers. --- **EGO-P-007 (unreachable files)** This one I think is a false flag from how the scanner walks the import graph. This extension uses a plugin-style architecture for widgets. The widgets are not statically imported from `extension.js`. Instead, they are discovered by scanning the `widgets/` folder at runtime and then loaded with dynamic `import(\`file://${entryPath}`)`, where `entryPath`is a computed string. See`lib/shell/widgetRuntimeLoader.js`and`lib/prefsWidgetManagement.js`. Because the import path is dynamic, a static import-graph walker starting from `extension.js` / `prefs.js` basically has no way to follow it. มันไม่รู้ว่า runtime จะเลือก file ไหน. That's why most of the files in the finding are actually used, they're just one step further down in that dynamic loading chain: * `lib/calendarGridKit.js` → used by `widgets/calendar-big/widget.js` and `widgets/calendar-events/widget.js` * `lib/iconAccentColor.js`, `lib/architectWidgetKit.js` → used by `widgets/xtile/widget.js` and `widgets/geek-architect/widget.js` * `lib/mediaApi.js` → used by the four `media-player-*` widgets * `lib/systemMetricsApi.js` → used by a dozen or so system-monitor style widgets * `lib/systemCalendarEvents.js` → used by `calendar-events` and `calendar-mini-events` * `lib/utils.js`, `lib/widgetConfigDefaults.js` → used across most of the bundled widgets * `lib/shell/halfCircleGaugeKit.js`, `lib/shell/widgetTooltip.js` → same story, imported from `widget.js` files which themselves are only reachable dynamically The `i18n/*.js` locale files are basically the same thing, just one layer deeper. They're not imported by name anywhere. `i18n/index.js` decides which file to load based on the user's locale at runtime: `GLib.build_filenamev([dirPath, \`${locale}.js`])` and then does a dynamic import. So again, static scanner can't really see these files. `widgets/calendar-header/i18n/de.js` follows the same pattern, just scoped to that widget's own i18n folder. `widget-center-prefs-app.js` is a little different. This one isn't imported as a JS module at all. It's launched as a separate GJS subprocess: `Gio.Subprocess.new(["gjs", "-m", scriptPath, ...])` from `extension.js`. So naturally it won't show up in an import trace, because there is no `import` statement pointing to it. I understand why the static check can't see all of this without actually executing the code, but these aren't orphaned / dead files. They're genuinely used, just reached through runtime path construction instead of static imports. If useful, we can add a comment near each dynamic `import()` explaining which files / folders it can load. That should make it a bit easier for future review and avoid this kind of false positive again.
xenatt auto- rejected
Auto-rejected because of new version 4 was uploaded