The Wayback Machine - https://web.archive.org/web/20200601102238/https://github.com/mysqljs/mysql/issues/1755
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is a async/await syntax change planned for the near future? #1755

Open
JedatKinports opened this issue Jun 22, 2017 · 18 comments
Open

Is a async/await syntax change planned for the near future? #1755

JedatKinports opened this issue Jun 22, 2017 · 18 comments

Comments

@JedatKinports
Copy link

@JedatKinports JedatKinports commented Jun 22, 2017

Hello, I will be starting a new project in the upcoming month and i was wondering. Will be an async/await syntax change in the near future? The async/await syntax is really like heaven compared to callbacks in my opinion. Is this on the todo list for the near future or should we try to find other solutions?

@JedatKinports JedatKinports changed the title Is a async/await syntax change planned in the near future? Is a async/await syntax change planned for the near future? Jun 22, 2017
@sidorares
Copy link
Member

@sidorares sidorares commented Jun 22, 2017

there are few promise wrappers around, https://www.npmjs.com/package/promise-mysql for example. This is all you need to start using async/await

@JedatKinports
Copy link
Author

@JedatKinports JedatKinports commented Jun 22, 2017

@sidorares i saw that wrapper already just wondered if this will be added to this package someday. In the meantime i will probably end up using the package you mentioned ( https://github.com/lukeb-uk/node-promise-mysql ) and will just add the await "in-front" of the promise.

@sidorares
Copy link
Member

@sidorares sidorares commented Jun 22, 2017

there are pro and cons of having wrapper as part of the driver. I decided to include it in my - https://github.com/sidorares/node-mysql2#using-promise-wrapper but still not sure if that was good decision

@JedatKinports
Copy link
Author

@JedatKinports JedatKinports commented Jun 22, 2017

Could you elaborate a bit more why you think that it wasn't a good decision? The only bad thing i noticed so far is that a lot of packages do not support/use promises/await/async and are still stuck on callbacks and mixing those i don't like. You end up with too many small functions with callbacks and then it's easier to lose sight of the application flow. My opinion at least.

@sidorares
Copy link
Member

@sidorares sidorares commented Jun 22, 2017

I'm talking more from maintainer point of view - less confusion about promise / non-promise api etc. For most users all in one is a good solution

@dougwilson
Copy link
Member

@dougwilson dougwilson commented Jun 24, 2017

Yea, ideally the module would provide just a single API; right now this module (being a very low-level driver) strives to support as many Node.js versions as possible, and as such, serves the lowest-common-dominator. The other thing is that we would have to make some significant changes somehow, as the project uses Event Emitters, and returns Objects from the callback functions, and since the return value will become a Promise instead of a Query object, for example, the API will need to determine how should the sql text string from the ongoing query be provided to the calling user. Ideally, being a base driver, other modules should pop up on npm to wrap this module to provide more user-friendly interfaces, just like you don't want to directly interact with the libmysql library in C for the same reasons.

@JedatKinports
Copy link
Author

@JedatKinports JedatKinports commented Jun 24, 2017

@dougwilson great explanation. Would be nice to have these intentions for the package listed somewhere in the Readme.md page with some links to some wrappers.

@dougwilson
Copy link
Member

@dougwilson dougwilson commented Aug 3, 2017

Would be nice to have these intentions for the package listed somewhere in the Readme.md page with some links to some wrappers.

Definitely a good first contribution!

@sidorares
Copy link
Member

@sidorares sidorares commented Aug 3, 2017

@dougwilson I just had a look at how aws sdk added promise support to existing callback-style api and this is the pattern we could repeat:

   const conn = await mysql.createConnection(opts).promise();
   const data = await conn.query(sql).promise();

or alternatively agree on a standard official promise wrapper module that works with both mysql and mysql2

@jamg44
Copy link

@jamg44 jamg44 commented Aug 30, 2017

If you want you can use this wrapper:

// support promises in conn.query
const originalQueryMethod = connection.query.bind(connection);
connection.query = (sql, cb) => new Promise((resolve, reject) => {
    originalQueryMethod(sql, (e, rows, fields) => {
        if (cb) return cb(e, rows, fields);
        return e ? reject(e) : resolve({rows, fields});
    });
});

// now you can use .then(), callbacks or await inside async functions 
const {rows, fields} = await conn.query(sql);
@samcov
Copy link

@samcov samcov commented Oct 10, 2017

@sidorares has the best short term suggestion, the only other suggestion would be to have something like,

const data = await conn.queryAsync(sql);

So there is no confusion between backwards support and moving forward.

@ebakhtarov
Copy link

@ebakhtarov ebakhtarov commented Nov 18, 2017

An option is the built-in utility function util.promisify in Node:

const fn = util.promisify(connection.query).bind(connection);
const rows = await fn('SELECT col1, col2 FROM users WHERE email = ?', [email]);

@samcov
Copy link

@samcov samcov commented Nov 19, 2017

@ebakhtarov - That wasn't an option for me until last week when we moved to Node 8.6 on our production server.

This is absolutely a great option until native support is available... Thanks a lot!

@devmarkpro
Copy link

@devmarkpro devmarkpro commented Mar 30, 2018

I solved it for myself with this approach.
I put a wrapper around our database calls that returns a Promise and in the callback, I resolve or reject promise based on the situation.
I know it's a little tricky but It's the fastest way that I know to add promise to my database call.

getAll: function(tableName, callback) {
        callback = callbackSure(callback);
        return new Promise(function(resolve, reject) {
            pool.getConnection(function (error, connection) {
                var query = connection.query('SELECT * FROM ' + tableName, [], function (e, r) {
                    if (e) {
                        reject(e);
                        callback(e);
                        connection.release();  
                    } else {
                        resolve(r);
                        callback(null, r);
                        connection.release();
                    }
                });
            });
        })
    }

and calling above function just like this:

const cities = await db.getAll('cities');
// do whatever you want with your cities!!!
@jafl
Copy link

@jafl jafl commented Feb 1, 2019

I have seen some libraries solving the callback/promise dilemma by simply returning a Promise if no callback is provided. That way, there is only a single set of functions in the API.

@sagar-gavhane
Copy link

@sagar-gavhane sagar-gavhane commented Mar 9, 2019

@ebakhtarov thanks, Its working properly .

@ws-h-ono
Copy link

@ws-h-ono ws-h-ono commented Jul 9, 2019

What is the reason for keeping Promise version implemented?
Are people OK using callback on every query or add their own promise wrapper?

I don't like the idea of delegating it to a smaller package whose account may be more susceptible for being hacked when the user base for this package is quite large.

@om-mani-padme-hum
Copy link

@om-mani-padme-hum om-mani-padme-hum commented Oct 1, 2019

Here is one implementation approach that has been working for me, but I like the idea of using the same function better and returning a promise if no callback argument is used, though that looks to be more involved.

https://github.com/om-mani-padme-hum/mysql/blob/master/lib/Connection.js
https://github.com/om-mani-padme-hum/mysql/blob/master/lib/Pool.js

See added "await" methods for callback-equivalent functions, i.e. awaitBeginTransaction, awaitCommit, awaitQuery, awaitRollback, awaitGetConnection, awaitEnd, etc.

This is similar to the method I used for my own wrapper, which I packaged up as mysql-await:

https://github.com/om-mani-padme-hum/mysql-await
https://www.npmjs.com/package/mysql-await

I agree with those that think this would be a valuable addition to the package, despite it being low-level. It's fundamental to the use of the package for anyone taking advantage of async/await capability, which is going to be most people. Right now the market is there's a dozen half-complete wrappers out there with no real leader among them. Fewer untrusted modules the better.

Thanks for all you guys do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.