Skip to content
This repository was archived by the owner on Mar 20, 2024. It is now read-only.

Commit 1b1050b

Browse files
mgolCaerusKaru
mgol
authored and
CaerusKaru
committed
fix: resolve Preboot race conditions (#83)
Currently Preboot suffers from various race conditions: 1. It waits for the `document.body` to be available and then applies its logic. However, the application root may not be available at this point - see the issue #72. 2. Even if the application root exists when Preboot starts listening for events the server node may not be available in full yet. Since Preboot searches for nodes matching desired selectors inside of the existing `serverNode`, it may skip some of them if the server node hasn't loaded fully. This is especially common if the internet connection is slow. 3. Preboot waits for `body` in a tight loop, checking every 10 milliseconds. This starves the browser but may also be clamped by it to a larger delay (around 1 second); that's what happens in modern browsers if the tab where the page loads is inactive which is what happens if you open a link in a new tab in Chrome or Firefox. This means Preboot may start listening for events after Angular reports the app is stable and the `PrebootComplete` event fires. This then leaves the server node active, never transferring to the client one which makes for a broken site. To solve it, we're doing the following: 1. Since we want to attach event listeners as soon as possible when the server node starts being available (so that it can be shallow-cloned) we cannot wait for it to be available in full. Therefore, we switched to delegated event handlers on each of the app roots instead of directly on specific nodes. 2. As we support multiple app roots, to not duplicate large inline preboot code, we've split `getInlinePrebootCode` into two parts: function definitions to be put in `<head>` and the invocation separate for each app root put just after the app root opening tag. 3. To maintain children offset numbers (otherwise selectors won't match between the client & the server) we've removed the in-app-root script immediately when it starts executing; this won't stop the execution. `document.currentScript` is a good way to find the currently running script but it doesn't work in IE. A fallback behavior is applied to IE that leverages the fact that start scripts are invoked synchronously so the current script is the last one currently in the DOM. 4. We've removed `waitUntilReady` as there's no reliable way to wait; we'll invoke the init code immediately instead. 5. We've switched the overlay used for freeing the UI to attach to `document.documentElement` instead of `document.body` so that we don't have to wait for `body`. 6. The mentioned overlay is now created for each app root separately to avoid race conditions. Fixes #82 Fixes #72 BREAKING CHANGES: 1. When used in a non-Angular app, the code needs to be updated to use `getInlineDefinition` and `getInlineInvocation`; the previously defined `getInlinePrebootCode` has been removed. The `getInlineDefinition` output needs to be placed before any content user might interactive with, preferably in <head>. The `getInlineInvocation` output needs to be put just after the opening tag of each app root. Only `getInlineDefinition` needs to have options passed. An example expected (simplified) layout in EJS format: ```html <html> <head> <script><%= getInlineDefinition(options) %></script> </head> <body> <app1-root> <script><%= getInlineInvocation() %></script> <h2>App1 header</h2> <div>content</div> </app1-root> <app2-root> <script><%= getInlineInvocation() %></script> <h2>App2 header</h2> <span>content</span> </app2-root> </body> </html> ``` 2. The refactor breaks working with frameworks that replace elements like AngularJS with UI Router as it no longer uses serverSelector and clientSelector but expects its clientNode to remain in the DOM.
1 parent 6e8417e commit 1b1050b

11 files changed

+225
-233
lines changed

‎README.md‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,24 +46,45 @@ import { AppComponent } from './app.component';
4646
export class AppModule { }
4747
```
4848

49-
The key part here for preboot is to include `PrebootModule.withConfig({ appRoot: 'app-root' })` where the `appRoot`
50-
is the selector(s) to find the root of your application. The options you can pass into `withConfig()` are in the [PrebootOptions](#PrebootOptions) section below. In most cases, however, you will only need to specify the `appRoot`.
49+
The key part here for preboot is to include `PrebootModule.withConfig({ appRoot: 'app-root' })` where the `appRoot` is the selector(s) to find the root of your application. The options you can pass into `withConfig()` are in the [PrebootOptions](#PrebootOptions) section below. In most cases, however, you will only need to specify the `appRoot`.
5150

5251
#### Non-Angular Server Configuration
5352

54-
```
55-
import { getInlinePrebootCode } from 'preboot';
53+
```ts
54+
import { getInlineDefinition, getInlineInvocation } from 'preboot';
5655

5756
const prebootOptions = {}; // see PrebootRecordOptions section below
58-
const inlineCode = getInlinePrebootCode(prebootOptions);
57+
const inlineCodeDefinition = getInlineDefinition(prebootOptions);
58+
const inlineCodeInvocation = getInlineInvocation();
59+
60+
// now insert `inlineCodeDefinition` into a `<script>` tag in `<head>` and
61+
// an `inlineCodeInvocation` copy just after the opening tag of each app root
5962

60-
// now simply insert the inlineCode into the HEAD section of your server view
63+
```
6164

65+
```html
66+
<html>
67+
<head>
68+
<script><%= inlineCodeDefinition %></script>
69+
</head>
70+
<body>
71+
<app1-root>
72+
<script><%= inlineCodeInvocation %></script>
73+
<h2>App1 header</h2>
74+
<div>content</div>
75+
</app1-root>
76+
<app2-root>
77+
<script><%= inlineCodeInvocation %></script>
78+
<h2>App2 header</h2>
79+
<span>content</span>
80+
</app2-root>
81+
</body>
82+
</html>
6283
```
6384

6485
#### Non-Angular Browser Configuration
6586

66-
```
87+
```ts
6788
import { EventReplayer } from 'preboot';
6889

6990
const replayer = new EventReplayer();
@@ -92,7 +113,7 @@ however where the page continues to change in significant ways. Basically if you
92113
the page after bootstrap then you will see some jank unless you set `replay` to `false` and then trigger replay
93114
yourself once you know that all async events are complete.
94115

95-
To manually trigger replay, simply inject the EventReplayer like this:
116+
To manually trigger replay, inject the EventReplayer like this:
96117

97118
```
98119
import { Injectable } from '@angular/core';

‎src/lib/api/event.recorder.spec.ts‎

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,13 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88
import {getMockElement} from '../common/preboot.mocks';
9-
import {createBuffer, createListenHandler, getSelection} from './event.recorder';
10-
import {
11-
EventSelector,
12-
PrebootAppData,
13-
PrebootData,
14-
PrebootSelection,
15-
ServerClientRoot,
16-
} from '../common/preboot.interfaces';
9+
import {createBuffer, getSelection} from './event.recorder';
10+
import {PrebootSelection, ServerClientRoot} from '../common/preboot.interfaces';
1711

1812
describe('UNIT TEST event.recorder', function() {
1913
describe('createBuffer()', function() {
2014
it('should do nothing if serverNode empty', function () {
2115
const root = <ServerClientRoot> {
22-
serverSelector: 'body',
2316
serverNode: undefined
2417
};
2518

@@ -29,7 +22,6 @@ describe('UNIT TEST event.recorder', function() {
2922

3023
it('should clone the node and insert before', function () {
3124
const root = <ServerClientRoot> {
32-
serverSelector: 'div',
3325
serverNode: getMockElement()
3426
};
3527
const clientNode = {
@@ -48,7 +40,6 @@ describe('UNIT TEST event.recorder', function() {
4840

4941
it('should add the "ng-non-bindable" attribute to serverNode', function () {
5042
const root = <ServerClientRoot> {
51-
serverSelector: 'div',
5243
serverNode: getMockElement()
5344
};
5445

@@ -99,35 +90,4 @@ describe('UNIT TEST event.recorder', function() {
9990
expect(actual).toEqual(expected);
10091
});
10192
});
102-
103-
104-
describe('createListenHandler()', function () {
105-
it('should do nothing if not listening', function () {
106-
const prebootData: PrebootData = <PrebootData> {
107-
listening: false
108-
};
109-
const eventSelector: EventSelector = {
110-
selector: '',
111-
events: [''],
112-
preventDefault: true
113-
};
114-
const appData: PrebootAppData = {
115-
root: {
116-
serverSelector: '',
117-
serverNode: undefined
118-
},
119-
events: []
120-
};
121-
const event = {
122-
preventDefault: function () {}
123-
};
124-
const node = <Element>{};
125-
126-
spyOn(event, 'preventDefault');
127-
128-
const handler = createListenHandler(prebootData, eventSelector, appData, node);
129-
handler(event as Event);
130-
expect(event.preventDefault).not.toHaveBeenCalled();
131-
});
132-
});
13393
});

0 commit comments

Comments
 (0)