Throw when using Nodejs version in browser #249
Conversation
Codecov Report
@@ Coverage Diff @@
## 5.x #249 +/- ##
=======================================
Coverage 98.27% 98.27%
=======================================
Files 17 17
Lines 2149 2149
Branches 611 611
=======================================
Hits 2112 2112
Misses 37 37Continue to review full report at Codecov.
|
|
I suggest to be more strict and throw an error if we try to use the nodejs version in a browser |
|
@ballinette Why? Technically, the SDK works even if we use the Node version in the browser. It is a common practice in famous browser libraries like React, Redux, Vuejs, etc... To display warnings when something looks bad, but maybe the user just knows what he/she is doing. |
|
@xbill82 so I don't understand the goal here. Is there any acceptable use case that a browser app could use the nodejs version of the sdk instead of the built one ? |
xbill82
changed the title
| if (typeof window !== 'undefined' && typeof BUILT === 'undefined') { | ||
| throw new Error('It looks like you are using the Nodejs version of Kuzzle SDK ' + | ||
| 'in a browser. ' + | ||
| 'It is strongly recommended to use the browser-specific build instead. ' + |
There was a problem hiding this comment.
Since we now throw an error, we are more than "strongly recommending" to use the built-version of the SDK. :-)
| throw new Error('It looks like you are using the Nodejs version of Kuzzle SDK ' + | ||
| 'in a browser. ' + | ||
| 'It is strongly recommended to use the browser-specific build instead. ' + | ||
| 'Learn more at https://github.com/kuzzleio/sdk-javascript/tree/master#browser'); |
There was a problem hiding this comment.
I cannot find that anchor in our README file, either in master, 5.x or 6.x
There was a problem hiding this comment.
the anchor in the README is #javascript : https://github.com/kuzzleio/sdk-javascript#javascript
should rather be #browser, imho, so the link here is correct, bug we need to fix the Readme.
There was a problem hiding this comment.
|
Merging as 4 ✅ |
6 participants
By injecting a
BUILTconstant via Webpack, and by checking thewindowglobal, we are able to detect if the user is using the Nodejs version in the browser.