tristanls / dynamodb-lock-client

A general purpose distributed locking library built for AWS DynamoDB.
MIT License
50 stars 19 forks source link

Removed setImmediate references #33

Closed fpronto closed 3 years ago

fpronto commented 3 years ago

This pull request is to improve possible deprecated code. setImmediate isn't being implemented in chrome or firefox in the immediate time. That function can be changed to setTimeout(func, 0);

tristanls commented 3 years ago

It is not deprecated. setImmediate is a Node.js utility since v0.9.1. What you're asking for is compatiblity with browsers.

How are you dealing with crypto.randomBytes(64) in the browser? As far as I understand require("crypto") doesn't work.

fpronto commented 3 years ago

Sorry, you are right. I add the following to the webpack export module

{
  resolve: {
    fallback: {
      os: require.resolve('os-browserify/browser'),
      crypto: require.resolve('crypto-browserify'),
      stream: require.resolve('stream-browserify'),
      buffer: require.resolve('buffer'),
    },
  plugins: [
    new webpack.ProvidePlugin({
      process: 'process/browser',
    }),
    new webpack.ProvidePlugin({
      Buffer: ['buffer', 'Buffer'],
    })
  ]
}

Should I add some kind of fallback for that kind of packages? also, right now I don't know if I add every one of this lines because of this package

tristanls commented 3 years ago

🤔 The only reason setImmediate is used is to keep reading the workflow event emitter top-down. Instead of using setImmediate, the alternative would be to emit:

workflow.emit("start",
        {
            partitionID,
            sortID,
            owner: self._config.owner || `${pkg.name}@${pkg.version}_${os.userInfo().username}@${os.hostname()}`,
            retryCount: self._retryCount,
            guid: crypto.randomBytes(64)
        }
    )

after all the workflow.on registrations have been registered (instead of before), i.e. on the bottom instead of the top.

fpronto commented 3 years ago

I've made this package work without this pull request. I don't need it to be approved. We can close this if you don't want to work on Browser support.

I am rewriting the setImmediate function by doing the following in the entry point of my project:

window.setImmediate = (func) => setTimeout(func, 0); 

Otherwise, if you want me to continue the work on full browser support I can do it

tristanls commented 3 years ago

Released v0.7.3 which no longer uses setImmediate (or setTimeout)