Problem of usage in nodejs 14.5 with the lastest version of @deepstream/[email protected] about deepstream.io-client-js HOT 10 CLOSED

@jaime-ez also related to the last release 😅

from deepstream.io-client-js.

Ok, I can confirm the issue, it's present with node>12 and it's not so simple to solve.

Apparently there are problems when bundling the 'ws' package with webpack, found multiple issues in other repos with no clear solution nor explanation for the problem (websockets/ws#1126, websockets/ws#1538, websockets/ws#1220, webpack/webpack#1019). At least for me, I wasn't able to make it work properly. When I managed to get 'ws' bundled, I got other issues and the bundle size went from ~190kb to ~750kb

I propose 2 options:

1

• Since we want to provide one package for both browser/react-native and node users, and including 'ws' in the bundle causes a huge penalty for the bundle size for browser/react-native users, implement a postinstall script that will change the main file in package.json depending on a env variable passed along when installing the @deepstream/client package.
• I assume most of the users are using @deepstream/client in node, so we could default the package.json main to dist/src/deepstream.js
• If the browser/react-native is the target platform, the package should be installed as: DEEPSTREAM_ENV=browser npm install @deepstream/client and include a postinstall script that will check for this env variable and change the main file in package.json accordingly.

2

• Just change back the package.json main to dist/src/deepstream.js and give instructions on the readme and docs that for react-native and browser env the main file should be changed manually.

I prefer option 1...Let me know what you think and of course if there is another way to do it.

from deepstream.io-client-js.

I got this too: Error: Cannot find module 'ws'. Now I see why.

I prefer Option 1, with the remark, that main pointing to dist/src/deepstream.js would work in both node and browser environments out of the box but with the size penalty in browser env you already mentioned.

I don't get it how the postinstall script in browser env will solve the 'ws can't be bundled with webpack' issue. What would the postinstall script do in browser env if it can't bundle ws? That means the ds.min.js minified/bundled file will never be correct due to the ws issue.

Other remarks:
I seem to recall that @deepstream/client worked well with node v13, not sure this is related, nor I remember well.

from deepstream.io-client-js.

I prefer Option 1, with the remark, that main pointing to dist/src/deepstream.js would work in both node and browser environments out of the box but with the size penalty in browser env you already mentioned.

Yes it does work in node and browser, but not in react-native, that's why we changed the main file to the minified bundle in order to make it work in react native (and browser but didn't check at the moment that it will brake the node platform..my mistake).

In summary pointing the main file in package.json to:

• dist/bundle/ds.min.js: works in browser and react-native (we have Websockets available in both platforms), not in node (we need the ws module)
• dist/src/deepstream.js: works in node (we use the ws module) and browser (Websockets available), not in react-native (we need the bundled Buffer module, not available in the platform)

The postinstall script will not solve anything related to webpack, it will point the main file to the bundle or not depending on the use case. And as you mention, since for the web it will also work just using the dist/src/deepstream.js file we could just add the env variable for react native (the name is just descriptive of course) DEEPSTREAM_ENV=react-native npm install @deepstream/client and then the post install script will change the main file to the bundle.
This will make the "special" install a narrower use case thus not adding noise for the most common node and browser scenarios.

Comments?

from deepstream.io-client-js.

Its clear now.

I saw in websockets/ws#1126 that they work around the bundling using

// in webpack.config.js
externals: {
      bufferutil: "bufferutil",
      "utf-8-validate": "utf-8-validate",
    }

If we put this also into our webpack.prod.js wouldn't it solve the problem? (I assume you've already tried that.)

from deepstream.io-client-js.

Yes I did, It did bundle the package but other problems emerged. Requires more work.
And as we discussed, making the special install for react-native only won't change the bundle size nor add more dependencies.

@yasserf any comments?

from deepstream.io-client-js.

I prefer the first option, we could also fork this and make a new package for browser and react-native, in the future there might be more differences.

from deepstream.io-client-js.

Honestly don't mind whatever solution you think is best, as long as it doesn't break current support. Bundling has never been my thing 🙈

from deepstream.io-client-js.

All right. Pushed a new version v5.1.3, that points the main file to dist/src/deepstream.js and has a postinstall script that check for the above mentioned flag for ease of install in react-native. I'll close for now, please re-open if problem persists!

from deepstream.io-client-js.

One last comment regarding bundled files and react-native: #544

from deepstream.io-client-js.