Partial XHR polyfull for Node.js #6463

mramato · 2018-04-18T16:22:32Z

This is a continuation of #6454 with some minor cleanup and additions.

1. Add `FeatureDetection.isNodeJs` 2. Improve getAbsoluteUri under node. 3. Some additional clean up for Node-specific Resource path.

cesium-concierge · 2018-04-18T16:22:34Z

@mramato, thanks for the pull request! Maintainers, we have a signed CLA from @mramato, so you can review this at any time.

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

mramato · 2018-04-18T16:24:42Z

CC @jimmyangel this will be in the May 1st release.

hpinkos · 2018-04-18T16:26:09Z

Source/Core/FeatureDetection.js

+    var isNodeJsResult;
+    function isNodeJs() {
+        if (!defined(isNodeJsResult)) {
+            isNodeJsResult = typeof process === 'object' && Object.prototype.toString.call(process) === '[object process]'; // eslint-disable-line


What's the eslint-disable-line for? Can we do eslint-disable-line some-eslint-rule instead of disabling everything for this line?

nevermind I guess haha

process is only a global in node and this is browser code so we need to ignore it. I couldn't figure out how to just allow a global for one line.

hpinkos · 2018-04-18T16:30:22Z

@mramato are you going to add @jimmyangel to CONTRIBUTORS.md?

jimmyangel · 2018-04-18T20:32:46Z

Hi @hpinkos @mramato, I can work on a couple of tests but could you please tell me where to plug them in so that they executed under nodejs (as opposed to a browser)?

mramato · 2018-04-18T20:50:01Z

Hi @hpinkos @mramato, I can work on a couple of tests but could you please tell me where to plug them in so that they executed under nodejs (as opposed to a browser)?

Thanks for the offer, but we actually don't have any Node unit tests at this time, so there's no where for you to put them. We want to have them eventually, but the plan is to figure out how to leverage our existing tests in node rather than have a separate set of node-specific tests.

mramato · 2018-04-18T20:50:18Z

@mramato are you going to add @jimmyangel to CONTRIBUTORS.md?

Sorry, I thought he was already in there but didn't check, I'll add him

jimmyangel · 2018-04-18T21:42:12Z

Thanks for the offer, but we actually don't have any Node unit tests at this time, so there's no where for you to put them. We want to have them eventually, but the plan is to figure out how to leverage our existing tests in node rather than have a separate set of node-specific tests.

Ok. FYI, I have created a package that uses sampleTerrain in node as a utility (and library) to add elevation to arbitrary GeoJSON data. It includes a few Jasmine tests that end up exercising the XHR functionality here. You could potential reuse, or simply add a command line invocation with a couple of test files. Just let me know if I can help.

jimmyangel · 2020-01-21T04:06:31Z

Hi @mramato,

It looks like the ability to use xhr from node.js broke in Cesium at some point :(. I tried to use Cesium.sampleTerrain from node.js, and I am seeing this:

TypeError: nodeRequire is not a function
    at loadWithHttpRequest (/Users/morin_ricardo/Projects/sampleterrain/node_modules/cesium/Source/Core/Resource.js:1892:19)
    at Object._34e‍.r.Resource._Implementations.loadWithXhr (/Users/morin_ricardo/Projects/sampleterrain/node_modules/cesium/Source/Core/Resource.js:1945:13)
    at Request.request.requestFunction (/Users/morin_ricardo/Projects/sampleterrain/node_modules/cesium/Source/Core/Resource.js:1298:49)
    at startRequest (/Users/morin_ricardo/Projects/sampleterrain/node_modules/cesium/Source/Core/RequestScheduler.js:195:17)
    at Function._34e‍.r.RequestScheduler.request (/Users/morin_ricardo/Projects/sampleterrain/node_modules/cesium/Source/Core/RequestScheduler.js:338:20)
    at Resource._34e‍.r.Resource._makeRequest (/Users/morin_ricardo/Projects/sampleterrain/node_modules/cesium/Source/Core/Resource.js:1307:40)
    at Resource._34e‍.r.Resource.fetch (/Users/morin_ricardo/Projects/sampleterrain/node_modules/cesium/Source/Core/Resource.js:1414:21)
    at Resource._34e‍.r.Resource.fetchJson (/Users/morin_ricardo/Projects/sampleterrain/node_modules/cesium/Source/Core/Resource.js:1091:28)
    at Function._34e‍.r.IonResource.fromAssetId (/Users/morin_ricardo/Projects/sampleterrain/node_modules/cesium/Source/Core/IonResource.js:98:33)
    at Proxy.createWorldTerrain (/Users/morin_ricardo/Projects/sampleterrain/node_modules/cesium/Source/Core/createWorldTerrain.js:37:30)

Looks like global.require is undefined in Resource.js. At some point it seems like it got messed up. Would this be something you guys could easily fix?

I propose replacing line 1905

var nodeRequire = global.require; // eslint-disable-line

with

var nodeRequire = global.require ? global.require : require; // eslint-disable-line

Does that work for you?

Thank you,

Ricardo

mramato · 2020-01-21T13:21:53Z

@jimmyangel Can you please write up a fresh issue for us to take a look at this. Thanks.

jimmyangel and others added 9 commits April 16, 2018 15:06

Reapply loadWithHttpRequest

21441b9

Build

e4a3c19

Handle responseType text

6046a06

Deal with document in node

a43aec9

Fix eslint

cd244df

Update CHANGES

ab717f2

Remove stuff carried from build

306e2ce

Streamline response decoding

41f5e15

Improve Node.js support

d057345

1. Add `FeatureDetection.isNodeJs` 2. Improve getAbsoluteUri under node. 3. Some additional clean up for Node-specific Resource path.

mramato mentioned this pull request Apr 18, 2018

Load with http request #6454

Closed

mramato merged commit 570b6f6 into master Apr 18, 2018

mramato deleted the node-xhr-polyfill branch April 18, 2018 16:24

hpinkos reviewed Apr 18, 2018

View reviewed changes

jimmyangel mentioned this pull request Jan 21, 2020

Regression: Cesium.sampleTerrain no longer usable from Node.js #8559

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partial XHR polyfull for Node.js #6463

Partial XHR polyfull for Node.js #6463

mramato commented Apr 18, 2018

cesium-concierge commented Apr 18, 2018

mramato commented Apr 18, 2018

hpinkos Apr 18, 2018

hpinkos Apr 18, 2018

mramato Apr 18, 2018

hpinkos commented Apr 18, 2018

jimmyangel commented Apr 18, 2018

mramato commented Apr 18, 2018

mramato commented Apr 18, 2018

jimmyangel commented Apr 18, 2018

jimmyangel commented Jan 21, 2020 •

edited

Loading

mramato commented Jan 21, 2020

Partial XHR polyfull for Node.js #6463

Partial XHR polyfull for Node.js #6463

Conversation

mramato commented Apr 18, 2018

cesium-concierge commented Apr 18, 2018

mramato commented Apr 18, 2018

hpinkos Apr 18, 2018

Choose a reason for hiding this comment

hpinkos Apr 18, 2018

Choose a reason for hiding this comment

mramato Apr 18, 2018

Choose a reason for hiding this comment

hpinkos commented Apr 18, 2018

jimmyangel commented Apr 18, 2018

mramato commented Apr 18, 2018

mramato commented Apr 18, 2018

jimmyangel commented Apr 18, 2018

jimmyangel commented Jan 21, 2020 • edited Loading

mramato commented Jan 21, 2020

jimmyangel commented Jan 21, 2020 •

edited

Loading