-
Notifications
You must be signed in to change notification settings - Fork 35
utils/httpRequestor.js should use relative path #70
Comments
Hey there, The structure of these internal packages is adding some confusion, I think, partially because - as you noted - the 'request' package in the SDK has a name collision with the 'request' library. The 'request' package in the SDK isn't the HTTP client that the rest of the SDK should use; rather, it's another set of SDK endpoints, much like These endpoints allow you to make Smartsheet requests to arbitrary endpoints while still leveraging your credentials and the SDK's error handling, etc., which is useful in case a feature is released to the Smartsheet API but has not yet been offered through the SDK yet. These methods are available through similar calls, such as This feature doesn't seem to have been documented yet in the repository README; I'll take that on as a task for us to complete. |
Hi @armstnp I seem to be getting an error which might be related to this issue. I'm getting the following error when I load my app:
Interestingly, I can make this error go away on npm start if I simply require smartsheet into my front-end app even though I don't need it there. I simply add:
But, even after that, the error still pops up in the terminal console every time my hot-reloader kicks in after making an edit locally. Any idea why this "request" reference might be getting messed up in your httpRequestor.js file? Thanks! --edit: I just took a closer look at your package.json. It looks like you're depending on the 'browser-request' package, but that is not included in your dependencies, is that the problem? (I think this is how "browser" works in package.json, but I'm not 100% sure...) smartsheet-javascript-sdk/package.json Line 28 in 9a8c1f8
It seems I can fix this for now by adding Cheers |
I don't believe the package was originally made with browser use in mind, so it could be that some things will need to be modified to work correctly if that's the environment you're running it in. I'm no longer a maintainer of this library, so hopefully that's an accurate description. Are you seeing this when using Smartsheet as a client-side library, or in Node? |
I'm seeing it when I use it in a node in a universal-rendering environment. I'm primarily using it on the node back-end, but I think our internal admin pages may be hitting it from the client-side, I'd have to check more carefully. Specifically, I'm using https://github.com/jaredpalmer/razzle with hot reloading. It seems to have fixed the error by adding 'browser-request' to my package dependencies. |
hi! I am having the same issue with httpReqestor, it is saying 'request' not found in smartsheet/lib/utils/httpReqestor when I change to the relative path of request (as this thread suggests to do) the error method becomes: "./{local path}/token_priv.json When I check package.json, the browser dependency is already as you said it should be. Anyone else hit this wall of errors? no clue how to get through! |
The "require("request")" assumes by default that is the "result" npm package (https://www.npmjs.com/package/request) because NO relative path is used; therefore, "npm install" will automatically assume the wrong package; but even worse, the "npm start" can fail because it cannot find the "request" module because it is not looking at "../request/..."
I highly recommend to change (around line 13 and 14 in ./lib/utils/httpRequestors.js):
var request = requestorConfig.request || Promise.promisifyAll(require("request"), {multiArgs: true});
to
var request = requestorConfig.request || Promise.promisifyAll(require("../request"), {multiArgs: true});
The text was updated successfully, but these errors were encountered: