Skip to content

Commit 6f2aed0

Browse files
authored
chore: avoid process.nextTick() and setImmediate() COMPASS-7314 (#4974)
Replace, or where unused remove, usage of `process.nextTick()` and `setImmediate()`, both of which are Node.js-specific APIs for async deferral for which browser-compatible replacements with near-identical behavior exist. Add linting rules to prevent usage of these methods. There might be a way to scope these linting rules to only the packages that we intend to use in browser environments, or exclude them from applying to test files. However, that seems unnecessary, given that the replacements are no harder to use, so a consistent style across the entire Compass codebase seems preferable.
1 parent b57b250 commit 6f2aed0

File tree

9 files changed

+33
-21
lines changed

9 files changed

+33
-21
lines changed

configs/eslint-config-compass/index.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,18 @@ module.exports = {
6060
'error',
6161
{ root: path.resolve(__dirname, '..', '..') },
6262
],
63+
'no-restricted-syntax': [
64+
'error',
65+
{
66+
selector: 'CallExpression[callee.name="setImmediate"]',
67+
message: 'Use browser-compatible `setTimeout(...)` instead',
68+
},
69+
{
70+
selector:
71+
'CallExpression[callee.object.name="process"][callee.property.name="nextTick"]',
72+
message: 'Use browser-compatible `queueMicrotask(...)` instead',
73+
},
74+
],
6375
},
6476
env: {
6577
...shared.env,

packages/atlas-service/src/store/atlas-signin-reducer.spec.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,16 @@ describe('atlasSignInReducer', function () {
316316
});
317317

318318
it('should reject if provided signal was aborted', async function () {
319+
let resolveSignInCalled = () => {};
320+
const signInCalled: Promise<void> = new Promise(
321+
(resolve) => (resolveSignInCalled = resolve)
322+
);
319323
const mockAtlasService = {
320324
isAuthenticated: sandbox.stub().resolves(false),
321-
signIn: sandbox.stub().resolves({ sub: '1234' }),
325+
signIn: sandbox.stub().callsFake(() => {
326+
resolveSignInCalled();
327+
return { sub: '1234' };
328+
}),
322329
getUserInfo: sandbox.stub().resolves({ sub: '1234' }),
323330
emit: sandbox.stub(),
324331
};
@@ -337,6 +344,9 @@ describe('atlasSignInReducer', function () {
337344
expect(err).to.have.property('message', 'Aborted from outside');
338345
}
339346
expect(store.getState()).to.have.property('state', 'canceled');
347+
348+
// Ensure that we are not leaving a dangling store operation that would conflict with our mocks being reset.
349+
await signInCalled;
340350
});
341351
});
342352
});

packages/atlas-service/src/store/atlas-signin-reducer.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -399,9 +399,7 @@ const startAttempt = (fn: () => void): AtlasSignInThunkAction<AttemptState> => {
399399
// noop for the promise created by `finally`, original promise rejection
400400
// should be handled by the service user
401401
});
402-
setImmediate(function () {
403-
fn();
404-
});
402+
setTimeout(fn);
405403
return attempt;
406404
};
407405
};

packages/compass-crud/src/components/editable-document.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class EditableDocument extends React.Component<
8686
if (this.state.editing || this.state.deleting) {
8787
// If the underlying document changed, that means that the collection
8888
// contents have been refreshed. In that case, stop editing/deleting.
89-
setImmediate(() => {
89+
setTimeout(() => {
9090
this.setState({ editing: false, deleting: false });
9191
});
9292
}

packages/compass-crud/src/components/table-view/cell-editor.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,9 +507,9 @@ class CellEditor
507507
}
508508

509509
onAddField(...args: Parameters<CellEditorProps['addColumn']>) {
510-
// we have to setImmediate here otherwise there's an untraceable
510+
// we have to setTimeout here otherwise there's an untraceable
511511
// setState on unmounted component error
512-
setImmediate(() => {
512+
setTimeout(() => {
513513
// we explicitly stop editing first to prevent breaking the UI
514514
this.props.api.stopEditing();
515515
this.props.addColumn(...args);

packages/compass-schema/src/stores/store.js

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -195,13 +195,6 @@ const configureStore = (options = {}) => {
195195

196196
onSchemaSampled() {
197197
this.geoLayers = {};
198-
199-
process.nextTick(() => {
200-
this.globalAppRegistry.emit('compass:schema:schema-sampled', {
201-
...this.state,
202-
geo: this.geoLayers,
203-
});
204-
});
205198
},
206199

207200
geoLayerAdded(field, layer) {

packages/compass-shell/src/plugin.spec.jsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,9 @@ import { CompassShell } from './components/compass-shell';
77
import createPlugin from './plugin';
88
import CompassShellStore from './stores';
99

10-
function nextTick() {
11-
return new Promise((resolve) => process.nextTick(resolve));
12-
}
13-
10+
// Wait until a component is present that is rendered in a limited number
11+
// of microtask queue iterations. In particular, this does *not* wait for the
12+
// event loop itself to progress.
1413
async function waitForAsyncComponent(wrapper, Component, attempts = 10) {
1514
let current = 0;
1615
let result;
@@ -21,7 +20,7 @@ async function waitForAsyncComponent(wrapper, Component, attempts = 10) {
2120
if (result.length > 0) {
2221
return result;
2322
}
24-
await nextTick();
23+
await Promise.resolve(); // wait a microtask queue iteration
2524
}
2625
return result;
2726
}

packages/compass/src/main/window-manager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ function trackWindowEvents(electronApp: App) {
296296

297297
// causes focus to be emitted after blur, allowing us to track
298298
// when the focus moves from a Compass window to the other
299-
setImmediate(() => {
299+
setTimeout(() => {
300300
const focusAt = windowFocusedAt.get(win.webContents.id);
301301
const movedToOtherCompassWin = windowFocusedAt.size === 2;
302302
windowFocusedAt.delete(win.webContents.id);

packages/ssh-tunnel/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ export class SshTunnel extends EventEmitter {
9090
this.rawConfig.socks5Username === user &&
9191
this.rawConfig.socks5Password === pass;
9292
debug('validating auth parameters', success);
93-
process.nextTick(cb, success);
93+
queueMicrotask(() => cb(success));
9494
}
9595
)
9696
);

0 commit comments

Comments
 (0)