Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swap node internal packages out for fetch in Resource #11773

Merged
merged 4 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
- By default, the screen space camera controller will no longer go inside or under instances of `Cesium3DTileset`. [#11581](https://github.com/CesiumGS/cesium/pull/11581)
- This behavior can be disabled by setting `Cesium3DTileset.disableCollision` to true.
- This feature is enabled by default only for WebGL 2 and above, but can be enabled for WebGL 1 by setting the `enablePick` option to true when creating the `Cesium3DTileset`.
- Remove the need for node internal packages `http`, `https`, `url` and `zlib` in the `Resource` class. This means they do not need to be marked external by build tools anymore. [#11773](https://github.com/CesiumGS/cesium/pull/11773)
- This slightly changed the contents of the `RequestErrorEvent` error that is thrown in node environments when a request fails. The `response` property is now a [`Response`](https://developer.mozilla.org/en-US/docs/Web/API/Response) object instead of an [`http.IncomingMessage`](https://nodejs.org/docs/latest-v20.x/api/http.html#class-httpincomingmessage)

##### Additions :tada:

Expand Down
4 changes: 2 additions & 2 deletions Specs/test.cjs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
/*eslint-env node*/
"use strict";

// NodeJS smoke screen test

const assert = require("node:assert");
const {
Cartographic,
createWorldTerrainAsync,
sampleTerrain,
} = require("cesium");

// NodeJS smoke screen test

async function test() {
const provider = await createWorldTerrainAsync();
const results = await sampleTerrain(provider, 11, [
Expand Down
3 changes: 2 additions & 1 deletion Specs/test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@ async function test() {
assert(results[1].height < 10000);
}

test();
test()

3 changes: 0 additions & 3 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,6 @@ export async function runCoverage(options) {
sourcemap: true,
format: "esm",
target: "es2020",
external: ["https", "http", "url", "zlib"],
outfile: karmaBundle,
logLevel: "error", // print errors immediately, and collect warnings so we can filter out known ones
});
Expand All @@ -812,7 +811,6 @@ export async function runCoverage(options) {
sourcemap: true,
format: "esm",
target: "es2020",
external: ["https", "http", "url", "zlib"],
outfile: specListBundle,
plugins: [instrumentPlugin],
logLevel: "error", // print errors immediately, and collect warnings so we can filter out known ones
Expand Down Expand Up @@ -1697,7 +1695,6 @@ async function buildCesiumViewer() {
config.format = "iife";
// Configure Cesium base path to use built
config.define = { CESIUM_BASE_URL: `"."` };
config.external = ["https", "http", "url", "zlib"];
config.outdir = cesiumViewerOutputDirectory;
config.outbase = "Apps/CesiumViewer";
config.logLevel = "error"; // print errors immediately, and collect warnings so we can filter out known ones
Expand Down
99 changes: 30 additions & 69 deletions packages/engine/Source/Core/Resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -2045,17 +2045,6 @@ Resource.createImageBitmapFromBlob = function (blob, options) {
});
};

function decodeResponse(loadWithHttpResponse, responseType) {
switch (responseType) {
case "text":
return loadWithHttpResponse.toString("utf8");
case "json":
return JSON.parse(loadWithHttpResponse.toString("utf8"));
default:
return new Uint8Array(loadWithHttpResponse).buffer;
}
}

function loadWithHttpRequest(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data and overrideMimeType arguments are not used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is intentional. They were not used before and I aimed to preserve the existing behavior as close as possible.

I don't know exactly why they weren't used in the first place or why this only transforms text and json but the goal of this PR was not to re-write the capability of this function; just swap out the components it uses to make building easier.

url,
responseType,
Expand All @@ -2066,64 +2055,36 @@ function loadWithHttpRequest(
overrideMimeType
) {
// Note: only the 'json' and 'text' responseTypes transforms the loaded buffer
let URL;
let zlib;
Promise.all([import("url"), import("zlib")])
.then(([urlImport, zlibImport]) => {
URL = urlImport.parse(url);
zlib = zlibImport;

return URL.protocol === "https:" ? import("https") : import("http");
})
.then((http) => {
const options = {
protocol: URL.protocol,
hostname: URL.hostname,
port: URL.port,
path: URL.path,
query: URL.query,
method: method,
headers: headers,
};
http
.request(options)
.on("response", function (res) {
if (res.statusCode < 200 || res.statusCode >= 300) {
deferred.reject(
new RequestErrorEvent(res.statusCode, res, res.headers)
);
return;
}

const chunkArray = [];
res.on("data", function (chunk) {
chunkArray.push(chunk);
});
fetch(url, {
method,
headers,
})
.then(async (response) => {
if (!response.ok) {
const responseHeaders = {};
response.headers.forEach((value, key) => {
responseHeaders[key] = value;
});
deferred.reject(
new RequestErrorEvent(response.status, response, responseHeaders)
);
return;
}

res.on("end", function () {
// eslint-disable-next-line no-undef
const result = Buffer.concat(chunkArray);
if (res.headers["content-encoding"] === "gzip") {
zlib.gunzip(result, function (error, resultUnzipped) {
if (error) {
deferred.reject(
new RuntimeError("Error decompressing response.")
);
} else {
deferred.resolve(
decodeResponse(resultUnzipped, responseType)
);
}
});
} else {
deferred.resolve(decodeResponse(result, responseType));
}
});
})
.on("error", function (e) {
deferred.reject(new RequestErrorEvent());
})
.end();
switch (responseType) {
case "text":
deferred.resolve(response.text());
break;
case "json":
deferred.resolve(response.json());
break;
default:
deferred.resolve(new Uint8Array(await response.arrayBuffer()).buffer);
break;
}
})
.catch(() => {
deferred.reject(new RequestErrorEvent());
});
}

Expand Down Expand Up @@ -2226,7 +2187,7 @@ Resource._Implementations.loadWithXhr = function (
//or do not support the xhr.response property.
if (xhr.status === 204) {
// accept no content
deferred.resolve();
deferred.resolve(undefined);
} else if (
defined(response) &&
(!defined(responseType) || browserResponseType === responseType)
Expand Down
7 changes: 2 additions & 5 deletions scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ export async function bundleCesiumJs(options) {
buildConfig.entryPoints = ["Source/Cesium.js"];
buildConfig.minify = options.minify;
buildConfig.sourcemap = options.sourcemap;
buildConfig.external = ["https", "http", "url", "zlib"];
buildConfig.plugins = options.removePragmas ? [stripPragmaPlugin] : undefined;
buildConfig.write = options.write;
buildConfig.banner = {
Expand Down Expand Up @@ -361,7 +360,7 @@ export async function bundleWorkers(options) {
const workers = await globby(["packages/engine/Source/Workers/**"]);
const workerConfig = defaultESBuildOptions();
workerConfig.bundle = true;
workerConfig.external = ["http", "https", "url", "zlib", "fs", "path"];
workerConfig.external = ["fs", "path"];

if (options.iife) {
let contents = ``;
Expand Down Expand Up @@ -816,7 +815,6 @@ export async function bundleCombinedSpecs(options) {
sourcemap: true,
outdir: path.join("Build", "Specs"),
plugins: [externalResolvePlugin],
external: [`http`, `https`, `url`, `zlib`],
write: options.write,
});
}
Expand All @@ -843,7 +841,7 @@ export async function bundleTestWorkers(options) {
format: "esm",
sourcemap: true,
outdir: path.join("Build", "Specs", "TestWorkers"),
external: ["http", "https", "url", "zlib", "fs", "path"],
external: ["fs", "path"],
write: options.write,
});
}
Expand Down Expand Up @@ -960,7 +958,6 @@ async function bundleSpecs(options) {
format: "esm",
outdir: options.outdir,
sourcemap: true,
external: ["https", "http", "zlib", "url"],
target: "es2020",
write: write,
};
Expand Down