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

Nwjs upgrade #275

Merged
merged 48 commits into from
Nov 22, 2016
Merged

Nwjs upgrade #275

merged 48 commits into from
Nov 22, 2016

Conversation

bastimeyer
Copy link
Member

@bastimeyer bastimeyer commented Jul 22, 2016

Overview

This is an upgrade to the new and rewritten NW.js.
Previously, this was blocked by the nw-builder, which didn't support these new builds. The notification system and the whole build system needed to be rewritten in order to enable a smooth transition without any regressions.

Requires

#295 Transition from RequireJS to Webpack

Fixes

#207 Memory leaks related to the broken garbage collector of legacy NW.js (<=0.12.3)
#227 Rendering issues on Windows in a maximized window
#234 High CPU usage on MacOS
#277 HiDPI awareness on Linux
#282 Number input fields being broken on MacOS

Enables

#226 Periodical route refreshes (was blocked by memory leaks)

Checklist

  • Upgrade to NWjs 0.18.7
    This enables support for native notifications on MacOS under a chromium-flag. (see below)
  • Nw-builder
    • New NW.js build config
      Support downloading/building new NW.js releases
    • Build flavor selection
      Don't publish NW.js SDK-flavor builds.
    • Remove osx32 builds
      Not supported by NW.js anymore
  • Switch from RequireJS to Webpack
    Merge Switch from RequireJS to Webpack #295 for npm dependency support.
  • Translate modules to es2015
    In preparation of required code refactorings or other needed implementations. fb60c22
  • Desktop notifications
    Native desktop notifications have been removed from the new NW.js. This means that all notifications will appear as Chromium's "Rich notifications". There were plans by the NW.js devs to implement a Node.js addon for bringing this feature back, but the idea seems to have been abandoned. They are now waiting for the Chromium devs to implement native notifications for OSX instead, which is kind of a let down for users of other platforms.
    • Implement node-notifier
      Merge nwjs-upgrade...node-notifier
      • Refactor NotificationService
      • Notification preferences
        Implement settings for choosing between different kind of notifications: native (node-notifier), growl (node-notifier), rich notifications (chromium) and automatic (default).
      • Notification center (OSX)
        Find a way to resolve icon issues on OSX (terminal-notifier).
      • Toast notifications (Win 8+)
        The application-shortcut API has been silently removed from NW.js. This prevents us from creating a startmenu shortcut with the required AppUserModelId property, which is required by Windows for showing proper toast notifications. Those shortcuts can't be set without using another 3rd party binary helper (c6a579a).
        A PR on the node-notifier repo might solve this issue by replacing the Toaster dependency with SnoreToast, which automatically creates shortcuts with the needed property. Waiting for the PR to be merged and a new version to be released.
    • NEW PLAN: Use latest NW.js (>= 0.16.0) and Chromium's native notifications
      Merge nwjs-upgrade...nwjs-upgrade-latest
      • Refactor NotificationService
      • Notification preferences
      • Rewrite notification system of the old node-notifier branch
        Don't use the node-notifier dependency!
        The SnoreToast PR on the node-notifier repo didn't receive any updates in months...
        Rewrite the previously implemented system which was basically a node-notifier wrapper:
        • Implement custom SnoreToast system on Windows
          This enables toast notifications and resolves the start menu shortcut issue (see above).
          • Create SnoreToast repo with precompiled binaries
            LGPL license being used -> include source code (doesn't need to be included in the final app code).
          • Remove old start menu shortcut logic
            SnoreToast now does this.
        • Use Chromium's native notifications on MacOS
          This removes the terminal-notifier dependency (less bloat) and fixes the icon issue.
          • Find a way to disable the "settings" button on native notifications on MacOS
        • Use lib-notify on Linux
          Nothing special here... notify-send is already being used by node-notifier.
        • Use growly
          Already a dependency of node-notifier... Use it instead.
    • Platform dependent builds
      Re-organize builds, so that 3rd party binaries for other platforms/archs are not included in every build.
  • Data migration
    There were some issues with the data migration of legacy NW.js versions when using one of the new ones. Those have been resolved.
  • Rendering issues
    New Chromium version is causing some issues.

http://docs.nwjs.io/en/latest/For%20Users/Migration/From%200.12%20to%200.13/

# Conflicts:
#	package.json
Dev tools are still there, despite the `--disable-devtools` chromium arg
# Conflicts:
#	src/app/app.js
#	src/app/initializers/nwjs.js
#	src/app/initializers/window.js
#	src/app/nwjs/nwWindow.js
#	src/app/nwjs/setup/tray.js
#	src/app/services/NotificationService.js
Don't set custom properties on the NW.js Window object.

- Refactor and rename nwjs/nwWindow to nwjs/Window
  - Export public methods instead of adding those to the Window object
  - Remove setShowInTray method
- Refactor and rename nwjs/tray to nwjs/Tray
  - Export public methods instead of exporting an Ember class

Also
- Refactor nwjs/openBrowser to nwjs/Shell
- Rename nwjs/menu to nwjs/Menu and nwjs/clipboard to nwjs/Clipboard
- Move nwjs/setup/* to initializers/nwjs
- Use logic of nwjs/setup/integrations directly in initializers/nwjs
- Fix imports in various modules
by implementing a custom shortcut creation logic.
The AppID is missing from the shortcut though, and can't be added
without using another 3rd party executable. <3 Windows /s

This all will be removed again once node-notifier releases a new version
with a different toaster executable which will automatically create
shortcuts with AppIDs included. The PR for that has not been merged yet.
(cherry picked from commit b5941c5)
Chromium doesn't support ::before and ::after pseudo elements on
input[type="number"]::-webkit-inner-spin-button ShadowDOM elements
anymore. A custom implementation is needed.

Also add ES2015 shim modules for PhantomJS
# Conflicts:
#	src/app/components/modal/ModalLivestreamerComponent.js
#	src/app/nwjs/tray.js
#	src/app/services/LivestreamerService.js
#	src/app/services/NotificationService.js
- Add node-notifier as a dependency
- Add notify_provider property to settings
- Implement notification selection in the settings menu
- Implement various NotificationProvider classes
- Refactor NotificationService
- Add CFBundleIdentifier to Info.plist of OSX builds
- Add platform.isMountainLion
- Refactor resolvePath module

TODO:

- Cache NotificationProvider.test
- Exclude node-notifier binaries from unrelated platform builds
Force a redraw on each animation frame of the svg element
Update chrome-remote-interface to latest version
and rename everything to "chrome debugging protocol".
Use es6 promises instead of callbacks and make sure
that everything is correctly executed asynchronously.
# Conflicts:
#	src/config/main.json
- Rewrite NotificationProvider and all its inheriting classes
- Remove NotificationProviderNotificationCenter
- Rename NotificationProviderToast to NotificationProviderSnoreToast
- Implement NotificationProviderNative
- Refactor NotificationService
- Remove node-notifier dependency
- Add growly and snoretoast dependencies
- Add babel es6 classes/arrow-func devDependencies (bc of uglifyJS)
- Add support for arch-specific builds on Windows
NWjs has dropped osx32 support since v0.13.0
Fixes isDirty check of ObjectBuffer'd settings

The NumberFieldComponent treats all values as numbers. This was not true
for the old value binding of the <input type=number> element and the
string type was needed for the ObjectBuffer to work correctly (strict).
We're running tests in NW.js now and don't need polyfills anymore
@bastimeyer bastimeyer merged commit 558576c into master Nov 22, 2016
@bastimeyer bastimeyer deleted the nwjs-upgrade branch November 27, 2016 09:57
@bastimeyer bastimeyer mentioned this pull request Jan 21, 2017
4 tasks
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.

1 participant