From 4b3571626dd711a38bd597bd16d4cd043f2d1e8e Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 11 Apr 2021 16:37:15 +0200 Subject: [PATCH] Align nextTick behavior with abstract-leveldown By using `queueMicrotask()` in browsers, for a consistent experience regardless of bundler (and of runtime in the future, when we drop node 10). Webpack no longer shims node core modules. In theory bundlers can now skip shimming `process`, were it not for our `readable-stream` dependency which uses `process.nextTick()` at the time of writing and has more reason to keep doing so. In modules where the risk of timing issues (due to not having a common microtask scheduler between modules) is low, like here in `levelup`, I prefer the idiomatic `queueMicroTask()` function. Lastly, maybe streams are not here to stay. Async iterators are coming to the level ecosystem. Or maybe we'll switch to `streamx` which is planning to use `queueMicrotask()` as well. --- lib/levelup.js | 11 +++++++---- lib/next-tick-browser.js | 11 +++++++++++ lib/next-tick.js | 3 +++ package.json | 8 ++++++-- test/create-stream-vs-put-racecondition.js | 3 ++- test/idempotent-test.js | 3 ++- test/init-test.js | 5 +++-- test/read-stream-test.js | 3 ++- test/self.js | 2 +- test/snapshot-test.js | 3 ++- 10 files changed, 39 insertions(+), 13 deletions(-) create mode 100644 lib/next-tick-browser.js create mode 100644 lib/next-tick.js diff --git a/lib/levelup.js b/lib/levelup.js index aafa5a8c..cd64690c 100644 --- a/lib/levelup.js +++ b/lib/levelup.js @@ -12,6 +12,9 @@ const catering = require('catering') const getCallback = require('./common').getCallback const getOptions = require('./common').getOptions +// TODO: after we drop node 10, also use queueMicrotask() in node +const nextTick = require('./next-tick') + const WriteError = errors.WriteError const ReadError = errors.ReadError const NotFoundError = errors.NotFoundError @@ -46,7 +49,7 @@ function LevelUP (db, options, callback) { if (!db || typeof db !== 'object') { error = new InitializationError('First argument must be an abstract-leveldown compliant store') if (typeof callback === 'function') { - return process.nextTick(callback, error) + return nextTick(callback, error) } throw error } @@ -97,7 +100,7 @@ LevelUP.prototype.open = function (opts, callback) { } if (this.isOpen()) { - process.nextTick(callback, null, this) + nextTick(callback, null, this) return callback.promise } @@ -132,7 +135,7 @@ LevelUP.prototype.close = function (callback) { this.emit('closing') this.db = new DeferredLevelDOWN(this._db) } else if (this.isClosed()) { - process.nextTick(callback) + nextTick(callback) } else if (this.db.status === 'closing') { this.once('closed', callback) } else if (this._isOpening()) { @@ -299,7 +302,7 @@ LevelUP.prototype.type = 'levelup' function maybeError (db, callback) { if (!db._isOpening() && !db.isOpen()) { - process.nextTick(callback, new ReadError('Database is not open')) + nextTick(callback, new ReadError('Database is not open')) return true } } diff --git a/lib/next-tick-browser.js b/lib/next-tick-browser.js new file mode 100644 index 00000000..d924b409 --- /dev/null +++ b/lib/next-tick-browser.js @@ -0,0 +1,11 @@ +'use strict' + +const queueMicrotask = require('queue-microtask') + +module.exports = function (fn, ...args) { + if (args.length === 0) { + queueMicrotask(fn) + } else { + queueMicrotask(() => fn(...args)) + } +} diff --git a/lib/next-tick.js b/lib/next-tick.js new file mode 100644 index 00000000..cebc1fd8 --- /dev/null +++ b/lib/next-tick.js @@ -0,0 +1,3 @@ +'use strict' + +module.exports = process.nextTick diff --git a/package.json b/package.json index e63fff4a..197df46a 100644 --- a/package.json +++ b/package.json @@ -4,13 +4,16 @@ "description": "Fast & simple storage - a Node.js-style LevelDB wrapper", "license": "MIT", "main": "lib/levelup.js", + "browser": { + "./lib/next-tick.js": "./lib/next-tick-browser.js" + }, "scripts": { "test": "standard && hallmark && (nyc -s node test/self.js | faucet) && nyc report", "coverage": "nyc report --reporter=text-lcov | coveralls", "test-browsers": "airtap --verbose test/self.js > airtap.log", "test-browsers-local": "airtap -p local test/self.js", "hallmark": "hallmark --fix", - "dependency-check": "dependency-check --no-dev .", + "dependency-check": "dependency-check --no-dev -i queue-microtask .", "prepublishOnly": "npm run dependency-check" }, "files": [ @@ -25,7 +28,8 @@ "deferred-leveldown": "~5.3.0", "level-errors": "^3.0.0", "level-iterator-stream": "^5.0.0", - "level-supports": "^2.0.0" + "level-supports": "^2.0.0", + "queue-microtask": "^1.2.3" }, "devDependencies": { "after": "^0.8.2", diff --git a/test/create-stream-vs-put-racecondition.js b/test/create-stream-vs-put-racecondition.js index 28b18a78..aef97d99 100644 --- a/test/create-stream-vs-put-racecondition.js +++ b/test/create-stream-vs-put-racecondition.js @@ -1,4 +1,5 @@ const levelup = require('../lib/levelup') +const nextTick = require('../lib/next-tick') const memdown = require('memdown') const encdown = require('encoding-down') const after = require('after') @@ -23,7 +24,7 @@ function makeTest (test, encode, deferredOpen, delayedPut) { test(name, function (t) { const db = encode ? levelup(encdown(memdown())) : levelup(memdown()) - const delay = delayedPut ? process.nextTick : callFn + const delay = delayedPut ? nextTick : callFn run(t, db, !deferredOpen, delay) }) diff --git a/test/idempotent-test.js b/test/idempotent-test.js index 01159ea4..c3f070fa 100644 --- a/test/idempotent-test.js +++ b/test/idempotent-test.js @@ -1,4 +1,5 @@ const levelup = require('../lib/levelup.js') +const nextTick = require('../lib/next-tick') const memdown = require('memdown') const sinon = require('sinon') @@ -18,7 +19,7 @@ module.exports = function (test, testCommon) { // close needs to be idempotent too. db.close() - process.nextTick(db.close.bind(db)) + nextTick(db.close.bind(db)) } const db = levelup(memdown(), function () { diff --git a/test/init-test.js b/test/init-test.js index 24078341..af4f907a 100644 --- a/test/init-test.js +++ b/test/init-test.js @@ -1,4 +1,5 @@ const levelup = require('../lib/levelup') +const nextTick = require('../lib/next-tick') const memdown = require('memdown') module.exports = function (test, testCommon) { @@ -51,7 +52,7 @@ module.exports = function (test, testCommon) { const mem = memdown() mem._open = function (opts, cb) { - process.nextTick(cb, new Error('from underlying store')) + nextTick(cb, new Error('from underlying store')) } levelup(mem, function (err) { @@ -68,7 +69,7 @@ module.exports = function (test, testCommon) { const mem = memdown() mem._open = function (opts, cb) { - process.nextTick(cb, new Error('from underlying store')) + nextTick(cb, new Error('from underlying store')) } levelup(mem) diff --git a/test/read-stream-test.js b/test/read-stream-test.js index 5cf52dfe..14736f69 100644 --- a/test/read-stream-test.js +++ b/test/read-stream-test.js @@ -3,6 +3,7 @@ const bigBlob = Array.apply(null, Array(1024 * 100)).map(function () { return 'a const discardable = require('./util/discardable') const readStreamContext = require('./util/rs-context') const rsFactory = require('./util/rs-factory') +const nextTick = require('../lib/next-tick') module.exports = function (test, testCommon) { const createReadStream = rsFactory(testCommon) @@ -523,7 +524,7 @@ module.exports = function (test, testCommon) { const rs = createReadStream(db) .on('close', done) - process.nextTick(function () { + nextTick(function () { rs.destroy() }) }) diff --git a/test/self.js b/test/self.js index 05bf2dfc..66cccf6b 100644 --- a/test/self.js +++ b/test/self.js @@ -50,6 +50,6 @@ suite({ // Integration tests that can't use a generic testCommon.factory() require('./self/manifest-test') -if (!process.browser) { +if (typeof process === 'undefined' || !process.browser) { require('./browserify-test')(test) } diff --git a/test/snapshot-test.js b/test/snapshot-test.js index 5f89108a..d7412645 100644 --- a/test/snapshot-test.js +++ b/test/snapshot-test.js @@ -3,6 +3,7 @@ const trickle = require('trickle') const discardable = require('./util/discardable') const readStreamContext = require('./util/rs-context') const rsFactory = require('./util/rs-factory') +const nextTick = require('../lib/next-tick') module.exports = function (test, testCommon) { const createReadStream = rsFactory(testCommon) @@ -28,7 +29,7 @@ module.exports = function (test, testCommon) { done() }, 0.05)) - process.nextTick(function () { + nextTick(function () { // 3) Concoct and write new random data over the top of existing items. // If we're not using a snapshot then then we'd expect the test // to fail because it'll pick up these new values rather than the