eclipse-theia / theia

Eclipse Theia is a cloud & desktop IDE framework implemented in TypeScript.
http://theia-ide.org
Eclipse Public License 2.0
19.96k stars 2.5k forks source link

consider using beforeunload event sparsely #8595

Open akosyakov opened 4 years ago

akosyakov commented 4 years ago

Using unload and beforeunload events is not recommended: https://developers.google.com/web/updates/2018/07/page-lifecycle-api#the-beforeunload-event In order to detect last page interaction one can use pagehide and visibilitychange events instead: https://developers.google.com/web/updates/2018/07/page-lifecycle-api#advice-hidden beforeunload event should be installed only when there is unsaved changes and removed when there are not

akosyakov commented 4 years ago

Bad effects which it is causing: https://github.com/gitpod-io/gitpod/issues/1959

akosyakov commented 4 years ago

In order to reproduce with Theia:

kittaakos commented 3 years ago

I did a prototype, it's looking very promising, but it does not really work in electron. Although calling addUnsavedChanges on the lifecycle does prevent the electron window to reload/close, I could not customize the underlying beforeunload listener, so I was not able to show a custom modal dialog in electron. It works in the browser. I tried it with Brave. There is another, generic issue I have noticed; I never received the expected hidden state on page reload. I tried it with both browser and electron. Maybe I have overlooked something.

I used this service to wrap the lifecycle:

import { injectable } from 'inversify';
import { Event as TheiaEvent, Emitter } from '../common';

const lifecycle: Lifecycle = require('page-lifecycle/dist/lifecycle.es5');

/**
 * The API documentation is [here](https://github.com/GoogleChromeLabs/page-lifecycle#api).
 */
export interface Lifecycle {
    readonly state: LifecycleState;
    readonly pageWasDiscarded: boolean;
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    addEventListener(type: 'statechange', listener: (event: LifecycleEvent) => any): void;
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    removeListener(type: 'statechange', listener: (event: LifecycleEvent) => any): void;
    addUnsavedChanges(id: Object | Symbol): void;
    removeUnsavedChanges(id: Object | Symbol): void;
}

/**
 * See https://developers.google.com/web/updates/2018/07/page-lifecycle-api.
 */
export type LifecycleState = 'active' | 'passive' | 'hidden' | 'frozen' | 'terminated' | 'discarded';

export interface LifecycleEvent {
    readonly oldState: LifecycleState;
    readonly newState: LifecycleState;
    readonly originalEvent: Event;
}

@injectable()
export class PageLifecycle {

    protected readonly instance = lifecycle;
    protected readonly stateChangedEmitter = new Emitter<LifecycleEvent>();

    constructor() {
        this.instance.addEventListener('statechange', this.onStateChange.bind(this));
    }

    protected onStateChange(event: LifecycleEvent): void {
        this.stateChangedEmitter.fire(event);
    }

    get state(): LifecycleState {
        return this.instance.state;
    }

    get onStateChanged(): TheiaEvent<LifecycleEvent> {
        return this.stateChangedEmitter.event;
    }

    addUnsavedChanges(id: Object | Symbol): void {
        this.instance.addUnsavedChanges(id);
    }

    removeUnsavedChanges(id: Object | Symbol): void {
        this.instance.removeUnsavedChanges(id);
    }

}

and this simple logic to track dirty widgets and call addUnsavedChanges/removeUnsavedChanges accordingly:

protected trackDirtyState(widget: Widget): void {
    const saveable = Saveable.get(widget);
    if (!saveable) {
        return;
    }
    const toDisposeOnWidgetClose = new DisposableCollection();
    const widgetDisposeListener = () => toDisposeOnWidgetClose.dispose();
    toDisposeOnWidgetClose.pushAll([
        Disposable.create(() => widget.disposed.disconnect(widgetDisposeListener)),
        Disposable.create(() => this.pageLifecycle.removeUnsavedChanges(widget)),
        saveable.onDirtyChanged(() => {
            if (saveable.dirty) {
                this.pageLifecycle.addUnsavedChanges(widget);
            } else {
                this.pageLifecycle.removeUnsavedChanges(widget);
            }
        })
    ]);
    widget.disposed.connect(() => widgetDisposeListener);
}