CamilleLegeron
3 months ago
commit
d57f4ab99d
@ -1,8 +1,8 @@
|
||||
# Welcome to the contribution guide for Grist!
|
||||
|
||||
You are eager to contribute to Grist? That's awesome! See below some contributions you can make:
|
||||
- [translate](/documentation/translate.md)
|
||||
- [write tutorials and user documentation](https://github.com/gristlabs/grist-help)
|
||||
- [translate](/documentation/translations.md)
|
||||
- [write tutorials and user documentation](https://github.com/gristlabs/grist-help?tab=readme-ov-file#grist-help-center)
|
||||
- [develop](/documentation/develop.md)
|
||||
- [report issues or suggest enhancement](https://github.com/gristlabs/grist-core/issues/new)
|
||||
|
||||
|
||||
@ -0,0 +1,259 @@
|
||||
import { AppModel } from 'app/client/models/AppModel';
|
||||
import { createAppPage } from 'app/client/ui/createAppPage';
|
||||
import { pagePanels } from 'app/client/ui/PagePanels';
|
||||
import { BootProbeInfo, BootProbeResult } from 'app/common/BootProbe';
|
||||
import { removeTrailingSlash } from 'app/common/gutil';
|
||||
import { getGristConfig } from 'app/common/urlUtils';
|
||||
import { Disposable, dom, Observable, styled, UseCBOwner } from 'grainjs';
|
||||
|
||||
const cssBody = styled('div', `
|
||||
padding: 20px;
|
||||
overflow: auto;
|
||||
`);
|
||||
|
||||
const cssHeader = styled('div', `
|
||||
padding: 20px;
|
||||
`);
|
||||
|
||||
const cssResult = styled('div', `
|
||||
max-width: 500px;
|
||||
`);
|
||||
|
||||
/**
|
||||
*
|
||||
* A "boot" page for inspecting the state of the Grist installation.
|
||||
*
|
||||
* TODO: deferring using any localization machinery so as not
|
||||
* to have to worry about its failure modes yet, but it should be
|
||||
* fine as long as assets served locally are used.
|
||||
*
|
||||
*/
|
||||
export class Boot extends Disposable {
|
||||
|
||||
// The back end will offer a set of probes (diagnostics) we
|
||||
// can use. Probes have unique IDs.
|
||||
public probes: Observable<BootProbeInfo[]>;
|
||||
|
||||
// Keep track of probe results we have received, by probe ID.
|
||||
public results: Map<string, Observable<BootProbeResult>>;
|
||||
|
||||
// Keep track of probe requests we are making, by probe ID.
|
||||
public requests: Map<string, BootProbe>;
|
||||
|
||||
constructor(_appModel: AppModel) {
|
||||
super();
|
||||
// Setting title in constructor seems to be how we are doing this,
|
||||
// based on other similar pages.
|
||||
document.title = 'Booting Grist';
|
||||
this.probes = Observable.create(this, []);
|
||||
this.results = new Map();
|
||||
this.requests = new Map();
|
||||
}
|
||||
|
||||
/**
|
||||
* Set up the page. Uses the generic Grist layout with an empty
|
||||
* side panel, just for convenience. Could be made a lot prettier.
|
||||
*/
|
||||
public buildDom() {
|
||||
const config = getGristConfig();
|
||||
const errMessage = config.errMessage;
|
||||
if (!errMessage) {
|
||||
// Probe tool URLs are relative to the current URL. Don't trust configuration,
|
||||
// because it may be buggy if the user is here looking at the boot page
|
||||
// to figure out some problem.
|
||||
const url = new URL(removeTrailingSlash(document.location.href));
|
||||
url.pathname += '/probe';
|
||||
fetch(url.href).then(async resp => {
|
||||
const _probes = await resp.json();
|
||||
this.probes.set(_probes.probes);
|
||||
}).catch(e => reportError(e));
|
||||
}
|
||||
|
||||
const rootNode = dom('div',
|
||||
dom.domComputed(
|
||||
use => {
|
||||
return pagePanels({
|
||||
leftPanel: {
|
||||
panelWidth: Observable.create(this, 240),
|
||||
panelOpen: Observable.create(this, false),
|
||||
hideOpener: true,
|
||||
header: null,
|
||||
content: null,
|
||||
},
|
||||
headerMain: cssHeader(dom('h1', 'Grist Boot')),
|
||||
contentMain: this.buildBody(use, {errMessage}),
|
||||
});
|
||||
}
|
||||
),
|
||||
);
|
||||
return rootNode;
|
||||
}
|
||||
|
||||
/**
|
||||
* The body of the page is very simple right now, basically a
|
||||
* placeholder. Make a section for each probe, and kick them off in
|
||||
* parallel, showing results as they come in.
|
||||
*/
|
||||
public buildBody(use: UseCBOwner, options: {errMessage?: string}) {
|
||||
if (options.errMessage) {
|
||||
return cssBody(cssResult(this.buildError()));
|
||||
}
|
||||
return cssBody([
|
||||
...use(this.probes).map(probe => {
|
||||
const {id} = probe;
|
||||
let result = this.results.get(id);
|
||||
if (!result) {
|
||||
result = Observable.create(this, {});
|
||||
this.results.set(id, result);
|
||||
}
|
||||
let request = this.requests.get(id);
|
||||
if (!request) {
|
||||
request = new BootProbe(id, this);
|
||||
this.requests.set(id, request);
|
||||
}
|
||||
request.start();
|
||||
return cssResult(
|
||||
this.buildResult(probe, use(result), probeDetails[id]));
|
||||
}),
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* This is used when there is an attempt to access the boot page
|
||||
* but something isn't right - either the page isn't enabled, or
|
||||
* the key in the URL is wrong. Give the user some information about
|
||||
* how to set things up.
|
||||
*/
|
||||
public buildError() {
|
||||
return dom(
|
||||
'div',
|
||||
dom('p',
|
||||
'A diagnostics page can be made available at:',
|
||||
dom('blockquote', '/boot/GRIST_BOOT_KEY'),
|
||||
'GRIST_BOOT_KEY is an environment variable ',
|
||||
' set before Grist starts. It should only',
|
||||
' contain characters that are valid in a URL.',
|
||||
' It should be a secret, since no authentication is needed',
|
||||
' to visit the diagnostics page.'),
|
||||
dom('p',
|
||||
'You are seeing this page because either the key is not set,',
|
||||
' or it is not in the URL.'),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* An ugly rendering of information returned by the probe.
|
||||
*/
|
||||
public buildResult(info: BootProbeInfo, result: BootProbeResult,
|
||||
details: ProbeDetails|undefined) {
|
||||
const out: (HTMLElement|string|null)[] = [];
|
||||
out.push(dom('h2', info.name));
|
||||
if (details) {
|
||||
out.push(dom('p', '> ', details.info));
|
||||
}
|
||||
if (result.verdict) {
|
||||
out.push(dom('pre', result.verdict));
|
||||
}
|
||||
if (result.success !== undefined) {
|
||||
out.push(result.success ? '✅' : '❌');
|
||||
}
|
||||
if (result.done === true) {
|
||||
out.push(dom('p', 'no fault detected'));
|
||||
}
|
||||
if (result.details) {
|
||||
for (const [key, val] of Object.entries(result.details)) {
|
||||
out.push(dom(
|
||||
'div',
|
||||
key,
|
||||
dom('input', dom.prop('value', JSON.stringify(val)))));
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Represents a single diagnostic.
|
||||
*/
|
||||
export class BootProbe {
|
||||
constructor(public id: string, public boot: Boot) {
|
||||
const url = new URL(removeTrailingSlash(document.location.href));
|
||||
url.pathname = url.pathname + '/probe/' + id;
|
||||
fetch(url.href).then(async resp => {
|
||||
const _probes: BootProbeResult = await resp.json();
|
||||
const ob = boot.results.get(id);
|
||||
if (ob) {
|
||||
ob.set(_probes);
|
||||
}
|
||||
}).catch(e => console.error(e));
|
||||
}
|
||||
|
||||
public start() {
|
||||
let result = this.boot.results.get(this.id);
|
||||
if (!result) {
|
||||
result = Observable.create(this.boot, {});
|
||||
this.boot.results.set(this.id, result);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a stripped down page to show boot information.
|
||||
* Make sure the API isn't used since it may well be unreachable
|
||||
* due to a misconfiguration, especially in multi-server setups.
|
||||
*/
|
||||
createAppPage(appModel => {
|
||||
return dom.create(Boot, appModel);
|
||||
}, {
|
||||
useApi: false,
|
||||
});
|
||||
|
||||
/**
|
||||
* Basic information about diagnostics is kept on the server,
|
||||
* but it can be useful to show extra details and tips in the
|
||||
* client.
|
||||
*/
|
||||
const probeDetails: Record<string, ProbeDetails> = {
|
||||
'boot-page': {
|
||||
info: `
|
||||
This boot page should not be too easy to access. Either turn
|
||||
it off when configuration is ok (by unsetting GRIST_BOOT_KEY)
|
||||
or make GRIST_BOOT_KEY long and cryptographically secure.
|
||||
`,
|
||||
},
|
||||
|
||||
'health-check': {
|
||||
info: `
|
||||
Grist has a small built-in health check often used when running
|
||||
it as a container.
|
||||
`,
|
||||
},
|
||||
|
||||
'host-header': {
|
||||
info: `
|
||||
Requests arriving to Grist should have an accurate Host
|
||||
header. This is essential when GRIST_SERVE_SAME_ORIGIN
|
||||
is set.
|
||||
`,
|
||||
},
|
||||
|
||||
'system-user': {
|
||||
info: `
|
||||
It is good practice not to run Grist as the root user.
|
||||
`,
|
||||
},
|
||||
|
||||
'reachable': {
|
||||
info: `
|
||||
The main page of Grist should be available.
|
||||
`
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* Information about the probe.
|
||||
*/
|
||||
interface ProbeDetails {
|
||||
info: string;
|
||||
}
|
||||
|
||||
@ -0,0 +1,22 @@
|
||||
|
||||
export type BootProbeIds =
|
||||
'boot-page' |
|
||||
'health-check' |
|
||||
'reachable' |
|
||||
'host-header' |
|
||||
'system-user'
|
||||
;
|
||||
|
||||
export interface BootProbeResult {
|
||||
verdict?: string;
|
||||
success?: boolean;
|
||||
done?: boolean;
|
||||
severity?: 'fault' | 'warning' | 'hmm';
|
||||
details?: Record<string, any>;
|
||||
}
|
||||
|
||||
export interface BootProbeInfo {
|
||||
id: BootProbeIds;
|
||||
name: string;
|
||||
}
|
||||
|
||||
@ -0,0 +1,185 @@
|
||||
import { ApiError } from 'app/common/ApiError';
|
||||
import { BootProbeIds, BootProbeResult } from 'app/common/BootProbe';
|
||||
import { removeTrailingSlash } from 'app/common/gutil';
|
||||
import { expressWrap, jsonErrorHandler } from 'app/server/lib/expressWrap';
|
||||
import { GristServer } from 'app/server/lib/GristServer';
|
||||
import * as express from 'express';
|
||||
import fetch from 'node-fetch';
|
||||
|
||||
/**
|
||||
* Self-diagnostics useful when installing Grist.
|
||||
*/
|
||||
export class BootProbes {
|
||||
// List of probes.
|
||||
public _probes = new Array<Probe>();
|
||||
|
||||
// Probes indexed by id.
|
||||
public _probeById = new Map<string, Probe>();
|
||||
|
||||
public constructor(private _app: express.Application,
|
||||
private _server: GristServer,
|
||||
private _base: string) {
|
||||
this._addProbes();
|
||||
}
|
||||
|
||||
public addEndpoints() {
|
||||
// Return a list of available probes.
|
||||
this._app.use(`${this._base}/probe$`, expressWrap(async (_, res) => {
|
||||
res.json({
|
||||
'probes': this._probes.map(probe => {
|
||||
return { id: probe.id, name: probe.name };
|
||||
}),
|
||||
});
|
||||
}));
|
||||
|
||||
// Return result of running an individual probe.
|
||||
this._app.use(`${this._base}/probe/:probeId`, expressWrap(async (req, res) => {
|
||||
const probe = this._probeById.get(req.params.probeId);
|
||||
if (!probe) {
|
||||
throw new ApiError('unknown probe', 400);
|
||||
}
|
||||
const result = await probe.apply(this._server, req);
|
||||
res.json(result);
|
||||
}));
|
||||
|
||||
// Fall-back for errors.
|
||||
this._app.use(`${this._base}/probe`, jsonErrorHandler);
|
||||
}
|
||||
|
||||
private _addProbes() {
|
||||
this._probes.push(_homeUrlReachableProbe);
|
||||
this._probes.push(_statusCheckProbe);
|
||||
this._probes.push(_userProbe);
|
||||
this._probes.push(_bootProbe);
|
||||
this._probes.push(_hostHeaderProbe);
|
||||
this._probeById = new Map(this._probes.map(p => [p.id, p]));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* An individual probe has an id, a name, an optional description,
|
||||
* and a method that returns a probe result.
|
||||
*/
|
||||
export interface Probe {
|
||||
id: BootProbeIds;
|
||||
name: string;
|
||||
description?: string;
|
||||
apply: (server: GristServer, req: express.Request) => Promise<BootProbeResult>;
|
||||
}
|
||||
|
||||
const _homeUrlReachableProbe: Probe = {
|
||||
id: 'reachable',
|
||||
name: 'Grist is reachable',
|
||||
apply: async (server, req) => {
|
||||
const url = server.getHomeUrl(req);
|
||||
try {
|
||||
const resp = await fetch(url);
|
||||
if (resp.status !== 200) {
|
||||
throw new ApiError(await resp.text(), resp.status);
|
||||
}
|
||||
return {
|
||||
success: true,
|
||||
};
|
||||
} catch (e) {
|
||||
return {
|
||||
success: false,
|
||||
details: {
|
||||
error: String(e),
|
||||
},
|
||||
severity: 'fault',
|
||||
};
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const _statusCheckProbe: Probe = {
|
||||
id: 'health-check',
|
||||
name: 'Built-in Health check',
|
||||
apply: async (server, req) => {
|
||||
const baseUrl = server.getHomeUrl(req);
|
||||
const url = new URL(baseUrl);
|
||||
url.pathname = removeTrailingSlash(url.pathname) + '/status';
|
||||
try {
|
||||
const resp = await fetch(url);
|
||||
if (resp.status !== 200) {
|
||||
throw new Error(`Failed with status ${resp.status}`);
|
||||
}
|
||||
const txt = await resp.text();
|
||||
if (!txt.includes('is alive')) {
|
||||
throw new Error(`Failed, page has unexpected content`);
|
||||
}
|
||||
return {
|
||||
success: true,
|
||||
};
|
||||
} catch (e) {
|
||||
return {
|
||||
success: false,
|
||||
error: String(e),
|
||||
severity: 'fault',
|
||||
};
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
const _userProbe: Probe = {
|
||||
id: 'system-user',
|
||||
name: 'System user is sane',
|
||||
apply: async () => {
|
||||
if (process.getuid && process.getuid() === 0) {
|
||||
return {
|
||||
success: false,
|
||||
verdict: 'User appears to be root (UID 0)',
|
||||
severity: 'warning',
|
||||
};
|
||||
} else {
|
||||
return {
|
||||
success: true,
|
||||
};
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
const _bootProbe: Probe = {
|
||||
id: 'boot-page',
|
||||
name: 'Boot page exposure',
|
||||
apply: async (server) => {
|
||||
if (!server.hasBoot) {
|
||||
return { success: true };
|
||||
}
|
||||
const maybeSecureEnough = String(process.env.GRIST_BOOT_KEY).length > 10;
|
||||
return {
|
||||
success: maybeSecureEnough,
|
||||
severity: 'hmm',
|
||||
};
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* Based on:
|
||||
* https://github.com/gristlabs/grist-core/issues/228#issuecomment-1803304438
|
||||
*
|
||||
* When GRIST_SERVE_SAME_ORIGIN is set, requests arriving to Grist need
|
||||
* to have an accurate Host header.
|
||||
*/
|
||||
const _hostHeaderProbe: Probe = {
|
||||
id: 'host-header',
|
||||
name: 'Host header is sane',
|
||||
apply: async (server, req) => {
|
||||
const host = req.header('host');
|
||||
const url = new URL(server.getHomeUrl(req));
|
||||
if (url.hostname === 'localhost') {
|
||||
return {
|
||||
done: true,
|
||||
};
|
||||
}
|
||||
if (String(url.hostname).toLowerCase() !== String(host).toLowerCase()) {
|
||||
return {
|
||||
success: false,
|
||||
severity: 'hmm',
|
||||
};
|
||||
}
|
||||
return {
|
||||
done: true,
|
||||
};
|
||||
},
|
||||
};
|
||||
@ -0,0 +1,148 @@
|
||||
# Disposal and Cleanup
|
||||
|
||||
Garbage-collected languages make you think that you don't need to worry about cleanup for your objects. In reality, there are still often cases when you do. This page gives some examples, and describes a library to simplify it.
|
||||
|
||||
## What's the problem
|
||||
|
||||
In the examples, we care about a situation when you have a JS object that is responsible for certain UI, i.e. DOM, listening to DOM changes to update state elsewhere, and listening to outside changes to update state to the DOM.
|
||||
|
||||
### DOM Elements
|
||||
So this JS object knows how to create the DOM. Removing the DOM, when the component is to be removed, is usually easy: `parentNode.removeNode(child)`. Since it's a manual operation, you may define some method to do this, named perhaps "destroy" or "dispose" or "cleanup".
|
||||
|
||||
If there is logic tied to your DOM either via JQuery events, or KnockoutJS bindings, you'll want to clean up the node specially: for JQuery, use `.remove()` or `.empty()` methods; for KnockoutJS, use `ko.removeNode()` or `ko.cleanNode()`. KnockoutJS's methods automatically call JQuery-related cleanup functions if JQuery is loaded in the page.
|
||||
|
||||
### Subscriptions and Computed Observables
|
||||
But there is more. Consider this knockout code, adapted from their simplest example of a computed observable:
|
||||
|
||||
function FullNameWidget(firstName, lastName) {
|
||||
this.fullName = ko.computed(function() {
|
||||
return firstName() + " " + lastName();
|
||||
});
|
||||
...
|
||||
}
|
||||
|
||||
Here we have a constructor for a component which takes two observables as constructor parameters, and creates a new observable which depends on the two inputs. Whenever `firstName` or `lastName` changes, `this.fullName` get recomputed. This makes it easy to create knockout-based bindings, e.g. to have a DOM element reflect the full name when either first or last name changes.
|
||||
|
||||
Now, what happens when this component is destroyed? It removes its associated DOM. Now when `firstName` or `lastName` change, there are no visible changes. But the function to recompute `this.fullName` still gets called, and still retains a reference to `this`, preventing the object from being garbage-collected.
|
||||
|
||||
The issue is that `this.fullName` is subscribed to `firstName` and `lastName` observables. It needs to be unsubscribed when the component is destroyed.
|
||||
|
||||
KnockoutJS recognizes it, and makes it easy: just call `this.firstName.dispose()`. We just have to remember to do it when we destroy the component.
|
||||
|
||||
This situation would exist without knockout too: the issue is that the component is listening to external changes to update the DOM that it is responsible for. When the component is gone, it should stop listening.
|
||||
|
||||
### Tying life of subscriptions to DOM
|
||||
Since the situation above is so common in KnockoutJS, it offers some assistance. Specifically, when a computed observable is created using knockout's own binding syntax (by specifying a JS expression in an HTML attribute), knockout will clean it up automatically when the DOM node is removed using `ko.removeNode()` or `ko.cleanNode()`.
|
||||
|
||||
Knockout also allows to tie other cleanup to DOM node removal, documented at [Custom disposal logic](http://knockoutjs.com/documentation/custom-bindings-disposal.html) page.
|
||||
|
||||
In the example above, you could use `ko.utils.domNodeDisposal.addDisposeCallback(node, function() { self.fullName.dispose(); })`, and when you destroy the component and remove the `node` via `ko.removeNode()` or `ko.cleanNode()`, the `fullName` observable will be properly disposed.
|
||||
|
||||
### Other knockout subscriptions
|
||||
There are other situations with subscriptions. For example, we may want to subscribe to a `viewId` observable, and when it changes, replace the currently-rendered View component. This might look like so
|
||||
|
||||
function GristDoc() {
|
||||
this.viewId = ko.observable();
|
||||
this.viewId.subscribe(function(viewId) {
|
||||
this.loadView(viewId);
|
||||
}, this);
|
||||
}
|
||||
|
||||
Once GristDoc is destroyed, the subscription to `this.viewId` still exists, so `this.viewId` retains a reference to `this` (for calling the callback). Technically, there is no problem: as long as there are no references to `this.viewId` from outside this object, the whole cycle should be garbage-collected.
|
||||
|
||||
But it's very risky: if anything else has a reference to `this.viewId` (e.g. if `this.viewId` is itself subscribed to, say, `window.history` changes), then the entire `GristDoc` is unavailable to garbage-collection, including all the DOM to which it probably retains references even after that DOM is detached from the page.
|
||||
|
||||
Beside the memory leak, it means that when `this.viewId` changes, it will continue calling `this.loadView()`, continuing to update DOM that no one will ever see. Over time, that would of course slow down the browser, but would be hard to detect and debug.
|
||||
|
||||
Again, KnockoutJS offers a way to unsubscribe: `.subscribe()` returns a `ko.subscription` object, which in turn has a `dispose()` method. We just need to call it, and the callback will be unsubscribed.
|
||||
|
||||
### Backbone Events
|
||||
|
||||
To be clear, the problem isn't with Knockout, it's with the idea of subscribing to outside events. Backbone allows listening to events, which creates the same problem, and Backbone offers a similar solution.
|
||||
|
||||
For example, let's say you have a component that listens to an outside event and does stuff. With a made-up example, you might have a constructor like:
|
||||
|
||||
function Game(basket) {
|
||||
basket.on('points:scored', function(team, points) {
|
||||
// Update UI to show updated points for the team.
|
||||
});
|
||||
}
|
||||
|
||||
Let's say that a `Game` object is destroyed, and a new one created, but the `basket` persists across Games. As the user continues to score points on the basket, the old (supposedly destroyed) Game object continues to have that inline callback called. It may not be showing anything, but only because the DOM it's updating is no longer attached to the page. It's still taking resources, and may even continue to send stuff to the server.
|
||||
|
||||
We need to clean up when we destroy the Game object. In this example, it's pretty annoying. We'd have to save the `basket` object and callback in member variables (like `this.basket`, `this.callback`), so that in the cleanup method, we could call `this.basket.off('points:scored', this.callback)`.
|
||||
|
||||
Many people have gotten bitten with that in Backbone (see this [stackoverflow post](http://stackoverflow.com/questions/14041042/backbone-0-9-9-difference-between-listento-and-on)) with a bunch of links to blog posts about it).
|
||||
|
||||
Backbone's solution is `listenTo()` method. You'd use it like so:
|
||||
|
||||
function Game(basket) {
|
||||
this.listenTo(basket, 'points:scored', function(team, points) {
|
||||
// Update UI to show updated points for the team.
|
||||
});
|
||||
}
|
||||
|
||||
Then when you destroy the Game object, you only have to call `this.stopListening()`. It keeps track of what you listened to, and unsubscribes. You just have to remember to call it. (Certain objects in Backbone will call `stopListening()` automatically when they are being cleaned up.)
|
||||
|
||||
### Internal events
|
||||
|
||||
If a component listens to an event on a DOM element it itself owns, and if it's using JQuery, then we don't need to do anything special. If on destruction of the component, we clean up the DOM element using `ko.removeNode()`, the JQuery event bindings should automatically be removed. (This hasn't been rigorously verified, but if correct, is a reason to use JQuery for browser events rather than native `addEventListener`.)
|
||||
|
||||
|
||||
## How to do cleanup uniformly
|
||||
|
||||
Since we need to destroy the components' DOM explicitly, the components should provide a method to call for that. By analogy with KnockoutJS, let's call it `dispose()`.
|
||||
|
||||
- We know that it needs to remove the DOM that the component is responsible for, probably using `ko.removeNode`.
|
||||
- If the component used Backbone's `listenTo()`, it should call `stopListening()` to unsubscribe from Backbone events.
|
||||
- If the component maintains any knockout subscriptions or computed observables, it should call `.dispose()` on them.
|
||||
- If the component owns other components, then those should be cleaned up recursively, by calling `.dispose()` on those.
|
||||
|
||||
The trick is how to make it easy to remember to do all necessary cleanup. I propose keeping track when the object to clean up first enters the picture.
|
||||
|
||||
## 'Disposable' class
|
||||
|
||||
The idea is to have a class that can be mixed into (or inherited by) any object, and whose purpose is to keep track of things this object "owns", that it should be responsible for cleaning up. To combine the examples above:
|
||||
|
||||
function Component(firstName, lastName, basket) {
|
||||
this.fullName = this.autoDispose(ko.computed(function() {
|
||||
return firstName() + " " + lastName();
|
||||
}));
|
||||
|
||||
this.viewId = ko.observable();
|
||||
this.autoDispose(this.viewId.subscribe(function(viewId) {
|
||||
this.loadView(viewId);
|
||||
}, this));
|
||||
|
||||
this.ourDom = this.autoDispose(somewhere.appendChild(some_dom_we_create));
|
||||
|
||||
this.listenTo(basket, 'points:scored', function(team, points) {
|
||||
// Update UI to show updated points for the team.
|
||||
});
|
||||
}
|
||||
|
||||
Note the `this.autoDispose()` calls. They mark the argument as being owned by `this`. When `this.dispose()` is called, those values get disposed of as well.
|
||||
|
||||
The disposal itself is fairly straightforward: if the object has a `dispose` method, we'll call that. If it's a DOM node, we'll call `ko.removeNode` on it. The `dispose()` method of Disposable objects will always call `this.stopListening()` if such a method exists, so that subscriptions using Backbone's `listenTo` are cleaned up automatically.
|
||||
|
||||
To do additional cleanup when `dispose()` is called, the derived class can override `dispose()`, do its other cleanup, then call `Disposable.prototype.dispose.call(this)`.
|
||||
|
||||
For convenience, Disposable class provides a few other methods:
|
||||
|
||||
- `disposeRelease(part)`: releases an owned object, so that it doesn't get auto-disposed.
|
||||
- `disposeDiscard(part)`: disposes of an owned object early (rather than wait for `this.dispose`).
|
||||
- `isDisposed()`: returns whether `this.dispose()` has already been called.
|
||||
|
||||
### Destroying destroyed objects
|
||||
|
||||
There is one more thing that Disposable class's `dispose()` method will do: destroy the object, as in ruin, wreck, wipe out. Specifically, it will go through all properties of `this`, and set each to a junk value. This achieves two goals:
|
||||
|
||||
1. In any of the examples above, if you forgot to mark anything with `this.autoDispose()`, and some callback continues to be called after the object has been destroyed, you'll get errors. Not just silent waste of resources that slow down the site and are hard to detect.
|
||||
|
||||
2. It removes references, potentially breaking references. Imagine that something wrongly retains a reference to a destroyed object (which logically nothing should, but something might by mistake). If it tries to use the object, it will fail (see point 1). But even if it doesn't access the object, it's preventing the garbage collector from cleaning any of the object. If we break references, then in this situation the GC can still collect all the properties of the destroyed object.
|
||||
|
||||
## Conclusion
|
||||
|
||||
All JS client-side components that need cleanup (e.g. maintain DOM, observables, listen to events, or subscribe to anything), should inherit from `Disposable`. To destroy them, call their `.dispose()` method. Whenever they take responsibility for any piece that requires cleanup, they should wrap that piece in `this.autoDispose()`.
|
||||
|
||||
This should go a long way towards avoiding leaks and slowdowns.
|
||||
@ -0,0 +1,42 @@
|
||||
# Migrations
|
||||
|
||||
If you change Grist schema, i.e. the schema of the Grist metadata tables (in `sandbox/grist/schema.py`), you'll have to increment the `SCHEMA_VERSION` (on top of that file) and create a migration. A migration is a set of actions that would get applied to a document at the previous version, to make it satisfy the new schema.
|
||||
|
||||
To add a migration, add a function to `sandbox/grist/migrations.py`, of this form (using the new version number):
|
||||
```lang=python
|
||||
@migration(schema_version=11)
|
||||
def migration11(tdset):
|
||||
return tdset.apply_doc_actions([
|
||||
add_column('_grist_Views_section', 'embedId', 'Text'),
|
||||
])
|
||||
```
|
||||
|
||||
Some migrations need to actually add or modify the data in a document. You can look at other migrations in that file for examples.
|
||||
|
||||
If you are doing anything other than adding a column or a table, you must read this document to the end.
|
||||
|
||||
## Philosophy of migrations
|
||||
|
||||
Migrations are tricky. Normally, we think about the software we are writing, but migrations work with documents that were created by an older version of the software, which may not have the logic you think our software has, and MAY have logic that the current version knows nothing about.
|
||||
|
||||
This is why migrations code uses its own "dumb" implementation for loading and examining data (see `sandbox/grist/table_data_set.py`), because trying to load an older document using our primary code base will usually fail, since the document will not satisfy our current assumptions.
|
||||
|
||||
## Restrictions
|
||||
|
||||
The rules below should make it at least barely possible to share documents by people who are not all on the same Grist version (even so, it will require more work). It should also make it somewhat safe to upgrade and then open the document with a previous version.
|
||||
|
||||
WARNING: Do not remove, modify, or rename metadata tables or columns.
|
||||
|
||||
Mark old columns and tables as deprecated using a comment. We may want to add a feature to mark them in code, to prevent their use in new versions. For now, it's enough to add a comment and remove references to the deprecated entities throughout code. An important goal is to prevent adding same-named entities in the future, or reusing the same column with a different meaning. So please add a comment of the form:
|
||||
|
||||
```lang=python
|
||||
# <columnName> is deprecated as of version XX. Do not remove or reuse.
|
||||
```
|
||||
|
||||
To justify keeping old columns around, consider what would happen if A (at version 10) communicates with B (at version 11). If column "foo" exists in v10, and is deleted in v11, then A may send actions that refer to "foo", and B would consider them invalid, since B's code has no idea what "foo" is. The solution is that B needs to still know about "foo", hence we don't remove old columns.
|
||||
|
||||
Similar justification applies to renaming columns, or modifying them (e.g. changing a type).
|
||||
|
||||
WARNING: If you change the meaning or type of a column, you have to create a new column with a new name.
|
||||
|
||||
You'll also need to write a migration to fill it from the old column, and would mark the old column as deprecated.
|
||||
@ -0,0 +1,15 @@
|
||||
<!doctype html>
|
||||
<html>
|
||||
<head>
|
||||
<meta charset="utf8">
|
||||
<!-- INSERT BASE -->
|
||||
<link rel="icon" type="image/x-icon" href="icons/favicon.png" />
|
||||
<link rel="stylesheet" href="icons/icons.css">
|
||||
<!-- INSERT LOCALE -->
|
||||
<!-- INSERT CONFIG -->
|
||||
<title>Loading...<!-- INSERT TITLE SUFFIX --></title>
|
||||
</head>
|
||||
<body>
|
||||
<script crossorigin="anonymous" src="boot.bundle.js"></script>
|
||||
</body>
|
||||
</html>
|
||||
@ -0,0 +1,52 @@
|
||||
import {assert, driver} from 'mocha-webdriver';
|
||||
import * as gu from 'test/nbrowser/gristUtils';
|
||||
import {server, setupTestSuite} from 'test/nbrowser/testUtils';
|
||||
import * as testUtils from 'test/server/testUtils';
|
||||
|
||||
describe('Boot', function() {
|
||||
this.timeout(30000);
|
||||
setupTestSuite();
|
||||
|
||||
let oldEnv: testUtils.EnvironmentSnapshot;
|
||||
|
||||
afterEach(() => gu.checkForErrors());
|
||||
|
||||
async function hasPrompt() {
|
||||
assert.include(
|
||||
await driver.findContentWait('p', /diagnostics page/, 2000).getText(),
|
||||
'A diagnostics page can be made available');
|
||||
}
|
||||
|
||||
it('gives prompt about how to enable boot page', async function() {
|
||||
await driver.get(`${server.getHost()}/boot`);
|
||||
await hasPrompt();
|
||||
});
|
||||
|
||||
describe('with a GRIST_BOOT_KEY', function() {
|
||||
before(async function() {
|
||||
oldEnv = new testUtils.EnvironmentSnapshot();
|
||||
process.env.GRIST_BOOT_KEY = 'lala';
|
||||
await server.restart();
|
||||
});
|
||||
|
||||
after(async function() {
|
||||
oldEnv.restore();
|
||||
await server.restart();
|
||||
});
|
||||
|
||||
it('gives prompt when key is missing', async function() {
|
||||
await driver.get(`${server.getHost()}/boot`);
|
||||
await hasPrompt();
|
||||
});
|
||||
|
||||
it('gives prompt when key is wrong', async function() {
|
||||
await driver.get(`${server.getHost()}/boot/bilbo`);
|
||||
await hasPrompt();
|
||||
});
|
||||
|
||||
it('gives page when key is right', async function() {
|
||||
await driver.get(`${server.getHost()}/boot/lala`);
|
||||
await driver.findContentWait('h2', /Grist is reachable/, 2000);
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Reference in new issue