Latest release should not be a patch!

[ChuckJonas]

Same as cometd/cometd#861

If you had a package that was relying on using this with a version previous to 4.0.4, it will automatically be upgraded and break things!

On that note, these might actually need major releases to prevent npm from auto-upgrading

[sbordet]

@ChuckJonas help me understand this.

An application that was using cometd-nodejs-client:1.0.2 and cometd:4.0.3 is working unmodified using cometd-nodejs-client:1.0.3 and cometd:4.0.4.
Because there were no API change and the application works unmodified, I made it a patch release.

[ChuckJonas]

The problem is is that if you had something like 1.0.x or ^1.0.3 in your package.json and you run npm install you will get upgraded to 1.0.4 automatically.

However, if your cometd version doesn't get updated to 4.0.4, it will break. I can understand your reasoning though, and maybe you're right. It's definitely my fault for not locking package version my in dependent packages. In my case, this change broke the last 4 releases (which I've now just deprecated).

But there is actually is a significant change to the way cometd:4.0.4 loads. You might actually call it a bug...

When you require/import it, it's immediately running the code to setup the runtime. The simple act of requiring a library should not throw an exception IMO. The ONLY way your node application will NOT crash is if you have the require('cometd-nodejs-client') BEFORE the require('cometd')

Steps to reproduce:

1. mkdir cometd-require-test
2. cd cometd-require-test
3. npm init accept defaults
4. npm install cometd cometd-nodejs-client
5. add index.js with the following code

require('cometd');
require('cometd-nodejs-client');

6. node index.js

EXPECTED
Should be able to import two libraries in any order, and it should work as long as cometd-nodejs-client is imported BEFORE attempting to initialize client

RESULT
Fails on line 1:

/Users/charlesjonas/Documents/code/comted-repo/node_modules/cometd/cometd.js:22
        runtime = window;
        ^

ReferenceError: window is not defined
    at _ids (/Users/charlesjonas/Documents/code/comted-repo/node_modules/cometd/cometd.js:22:9)
    at Object.<anonymous> (/Users/charlesjonas/Documents/code/comted-repo/node_modules/cometd/cometd.js:37:2)
    at Module._compile (internal/modules/cjs/loader.js:654:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:665:10)
    at Module.load (internal/modules/cjs/loader.js:566:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:506:12)
    at Function.Module._load (internal/modules/cjs/loader.js:498:3)
    at Module.require (internal/modules/cjs/loader.js:598:17)
    at require (internal/modules/cjs/helpers.js:11:18)
    at Object.<anonymous> (/Users/charlesjonas/Documents/code/comted-repo/index.js:1:63)

[sbordet]

@ChuckJonas it has always been the intention of cometd-nodejs-client to be required before cometd, since cometd has its root in browsers (not Node) and therefore assumes a number of JavaScript things that are available in browsers but not in Node.
cometd-nodejs-client was acting as "I'm faking the minimum stuff that cometd requires so that it'll believe it's running in a browser".
The initial version of this was to "fake" a window object, but then you filed an issue about that.
To break that dependency on window, I need to get globals such as setTimeout() and XMLHttpRequest, etc. from somewhere.
So either I pollute the global Node object (which I don't want to do it), or I don't but then I need something that runs before cometd that makes available what CometD requires - this I what I attempted with cometd-nodejs-client 1.0.3 and cometd 4.0.4.
Now I can be a tad smarter and avoid to use the window reference (using this instead), but the require order is necessary.

Unless you have other solutions that I can't see. Thanks!

And there are already people complaining that I'm using this.

[ChuckJonas]

@sbordet With 4.0.3, you could import cometd before cometd-nodejs-client. This was helpful to allow the consuming library to "patch" cometd during runtime, and ONLY when needed (IOW when we've detected a node ENV). This allows others to write isomorphic libraries (something that can run in the browser or in node).

It doesn't seem like there is any reason to run the code to setup cometdRuntime on import. Instead it should be postponed until a client is actually initialized for the first time.

In general running code on imports should be avoided, but if you don't think there is another way I understands

[sbordet]

The cometd package has been reverted back to the old code, so I believe this issue can be closed.