pg-query-stream version + compatibility

[vitaly-t]

pg-query-stream has been fully revamped in the latest release, with at least one breaking change - changing the class name again, from PgQueryStream to QueryStream (see PR #2476).

There are libraries out there that validate the object by its class name, so those would all need to be updated.

The request is here that pg-query-stream version should have been upped to 4.0.0, or not with the minor version increment.

Please, either bring the version to 4.0.0 (preferable), or rename the class back to PgQueryStream (this will require TypeScript update as well, so the first option is better).

---

Note that this is the second time such a change happens. Which means there should be a test added for the class name, to make sure people stop changing it. Internal class names are often important, being the only way an object instance can be validated.

[chyzwar]

@brianc I made that change. I saw that tests, documentation and definitely typed where using QueryStream as name. Let me know what do you think ?

[vitaly-t]

@chyzwar I think we should keep the class name as is, just up pg-query-stream package version to 4.0.0, and that's it. And perhaps add a test to make sure the class is never renamed again.

[charmander]

There are libraries out there that validate the object by its class name

That doesn’t sound like a good idea.

[brianc]

There are libraries out there that validate the object by its class name

Yeah there's an argument that you could say any change is a breaking change. What if an external library validated a function by the contents of calling .toString() on the function. Any lines of code in the function change...breaking change!

I'm fine to bump the version if there's a material issue caused by this, but I'd like to see an actual example in the wild before just calling this a breaking change because the name of a class changed. It sounds more like pedantry than an actual issue.

[vitaly-t]

@brianc Your pg-query-stream is for integration with third-party modules. How are those supposed to determine that the object they are passed in is indeed of type QueryStream? How to validate for that?

Class name is the only approach I found usable. If you have a better way, then please do post it here.

[brianc]

How to validate for that?

I would imagine something like...

const QueryStream = require('pg-query-stream')

if (something instanceof QueryStream) {
  // we have a stream!
}

that also works like this

const ICanCallThisAnythingIWant = require('pg-query-stream')
if (something instanceof ICanCallThisAnythingIWant) {
  // we have a stream!
}

Does that work?

[vitaly-t]

@brianc Nope, it doesn't. The modern NodeJS has gotten too progressive, one might say. These days you have things like docker that manages to isolate modules, so instanceof fails when applied across two isolated docker containers, even though the module is the same. So I'm trying to stay away from the instanceof approach, as it only works with the traditional uni-module approach.

Another issue - you have to include the actual module type into your project. But when you only add support for it, you do not really need its implementation, makes it way more flexible. And avoids versioning conflict.

[brianc]

Sorry - not following what you're saying about docker - how can you require modules from one docker container into another? I don't do super advanced stuff w/ docker.

Also, not following what you mean about including the actual module type in your project, could you show me a code sample or example here? I still think checking the name of a class and changing behavior based on that is a brittle approach to things, but I can do a semver major bump if required.

[vitaly-t]

Within docker, it is possible to load the same module multiple times, each within isolated environment, so if you create a class object in one, pass it into the other, then instanceof is suddenly not working, JavaScript sees it as 2 separate classes.

not following what you mean about including the actual module type in your project

When you want to add support for QueryStream, you shouldn't need to include the module itself. The latter is done by the module that uses your module. Including into module that doesn't really need it only creates problem - extra dependency, plus possible version conflict.

I still think checking the name of a class and changing behavior based on that is a brittle approach to things

I have explained the best I could why and why it doesn't work, so not much option there.

but I can do a semver major bump if required.

Yes, please, it would make life easier, since the last version suddenly changed the class name.