Skip to content
Open
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
7 changes: 7 additions & 0 deletions .changeset/six-trains-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@esri/arcgis-rest-request": minor
---

Refactored internal request option merging by moving into `mergeNormalizedRequestOptions`, and updated `appendCustomParams` to normalize legacy `ILegacyRequestOptions` into `IRequestOptions` (`requestFlags`/`fetchOptions`) before returning.

Added a documented `IRequestFlags` interface and improved option documentation to clarify legacy options are in maintenance mode and being phased out.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export async function registerApp(
stringifyArrays(options);

const url = getPortalUrl(options) + "/oauth2/registerApp";
options.httpMethod = "POST";
options.fetchOptions.method = "POST";
options.params.f = "json";

const registeredAppResponse: IRegisteredAppResponse = await request(
Expand Down
85 changes: 46 additions & 39 deletions packages/arcgis-rest-request/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export function setDefaultRequestOptions(
"You should not set `authentication` as a default in a shared environment such as a web server which will process multiple users requests. You can call `setDefaultRequestOptions` with `true` as a second argument to disable this warning."
);
}
warnOnDeprecatedRequestOptions(options, hideWarnings);
(globalThis as any).DEFAULT_ARCGIS_REQUEST_OPTIONS =
normalizeDeprecatedRequestOptions(options);
}
Expand All @@ -61,6 +62,7 @@ export function getDefaultRequestOptions(): IRequestOptions {
const defaultRequestOptions = (globalThis as any)
.DEFAULT_ARCGIS_REQUEST_OPTIONS;
if (defaultRequestOptions) {
warnOnDeprecatedRequestOptions(defaultRequestOptions, true);
return normalizeDeprecatedRequestOptions(defaultRequestOptions);
}
return {
Expand All @@ -73,6 +75,46 @@ export function getDefaultRequestOptions(): IRequestOptions {
};
}

/**
* Merges request options by translating legacy options to their normalized equivalents,
* applying defaults, and deeply merging params/requestFlags/fetchOptions.headers.
*/
function mergeNormalizedRequestOptions(
requestOptions: IRequestOptions,
defaultRequestOptions: IRequestOptions
): IRequestOptions {
const suppressWarnings =
requestOptions.requestFlags?.suppressWarnings ??
requestOptions.suppressWarnings;
warnOnDeprecatedRequestOptions(requestOptions, suppressWarnings);

const normalizedRequestOptions =
normalizeDeprecatedRequestOptions(requestOptions);

return {
...defaultRequestOptions,
...normalizedRequestOptions,
...{
params: {
...defaultRequestOptions.params,
...normalizedRequestOptions.params
},
requestFlags: {
...defaultRequestOptions.requestFlags,
...normalizedRequestOptions.requestFlags
},
fetchOptions: {
...defaultRequestOptions.fetchOptions,
...normalizedRequestOptions.fetchOptions,
headers: {
...(defaultRequestOptions.fetchOptions?.headers as any),
...(normalizedRequestOptions.fetchOptions?.headers as any)
}
Comment on lines +109 to +112

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dixonyant I think that this needs to use the new merge header util that was added at some point. Unless normalizedRequestOptions and defaultRequestOptions both return the headers as an object?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually looking at this is is normalize to an object so this works.

}
}
};
}

/**
* This error is thrown when a request encounters an invalid token error. Requests that use {@linkcode ArcGISIdentityManager} or
* {@linkcode ApplicationCredentialsManager} in the `authentication` option the authentication manager will automatically try to generate
Expand Down Expand Up @@ -213,44 +255,6 @@ export function checkForErrors(
return response;
}

function normalizeRequestOptions(
requestOptions: IRequestOptions
): IRequestOptions {
const suppressWarnings =
requestOptions.requestFlags?.suppressWarnings ??
requestOptions.suppressWarnings ??
false;
warnOnDeprecatedRequestOptions(requestOptions, suppressWarnings);

const normalizedRequestOptions =
normalizeDeprecatedRequestOptions(requestOptions);
const defaults = getDefaultRequestOptions();

return {
...{ fetchOptions: { method: "POST" } },
...defaults,
...normalizedRequestOptions,
...{
params: {
...defaults.params,
...normalizedRequestOptions.params
},
requestFlags: {
...defaults.requestFlags,
...normalizedRequestOptions.requestFlags
},
fetchOptions: {
...defaults.fetchOptions,
...normalizedRequestOptions.fetchOptions,
headers: mergeHeaders(
defaults.fetchOptions?.headers,
normalizedRequestOptions.fetchOptions?.headers
)
}
}
};
}

function buildAuthenticationManager(
options: IRequestOptions
): IAuthenticationManager | undefined {
Expand Down Expand Up @@ -356,7 +360,10 @@ async function executeRequest(
options: IRequestOptions;
originalAuthError: ArcGISAuthError | null;
}> {
const options = normalizeRequestOptions(requestOptions);
const options = mergeNormalizedRequestOptions(
requestOptions,
getDefaultRequestOptions()
);

const params: IParams = {
...{ f: "json" },
Expand Down
46 changes: 38 additions & 8 deletions packages/arcgis-rest-request/src/utils/IRequestOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { IAuthenticationManager } from "./IAuthenticationManager.js";
/**
* Legacy request option properties kept for backwards compatibility.
*
* @deprecated This class represents the v4 @linkcode{IRequestOptions} which should be used instead.
* @deprecated This interface contains deprecated properties from the REST JS v4 @linkcode{IRequestOptions}, use @linkcode{IRequestOptions} instead.
*/
export interface ILegacyRequestOptions {
/**
Expand Down Expand Up @@ -56,6 +56,39 @@ export interface ILegacyRequestOptions {
suppressWarnings?: boolean;
}

/**
* Internal flags that control ArcGIS REST JS request behavior.
*
*/
export interface IRequestFlags {
/**
* Attempts to keep authentication tokens out of URL query params.
*
* For GET requests in Node.js, REST JS may use the `X-Esri-Authorization` header.
* In browser flows where that header path is not used, REST JS may switch to POST
* so the token is sent in the request body instead of the URL.
*/
hideToken?: boolean;
/**
* Suppresses ArcGIS REST JS request warnings for this call, including deprecation
* and format-related warnings emitted during request processing.
*/
suppressWarnings?: boolean;
/**
* Reserved for response header injection behavior.
*
* Note: this flag is currently defined for API compatibility/documentation, but is
* not actively applied in the current request pipeline.
*/
injectRequestHeaders?: boolean;
/**
* Disables automatic GET -> POST conversion based solely on URL length (2000+ chars).
*
* Other request rules may still result in POST (for example token-hiding behavior).
*/
ignoreMaxUrlLength?: boolean;
}

/**
* Options for the `request()` method.
*/
Expand All @@ -73,13 +106,10 @@ export interface IRequestOptions extends ILegacyRequestOptions {
* exists, otherwise to 'https://www.arcgis.com/sharing/rest'.
*/
portal?: string;
requestFlags?: {
// additional options for our internal request method
hideToken?: boolean; // put the token param in the header for GET requests
suppressWarnings?: boolean; // silence all internal console warnings from REST JS
injectRequestHeaders?: boolean; // add a `arcgisRestRequestHeaders` property that returns the request headers to resolve https://github.com/Esri/arcgis-rest-js/issues/1181
ignoreMaxUrlLength?: boolean; // ignore the URL length and use specified HTTP method
};
/**
* ArcGIS REST JS runtime flags.
*/
requestFlags?: IRequestFlags;
/**
* anything you can pass to the options for fetch
* https://developer.mozilla.org/en-US/docs/Web/API/RequestInit
Expand Down
13 changes: 11 additions & 2 deletions packages/arcgis-rest-request/src/utils/append-custom-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@
* Apache-2.0 */

import { IRequestOptions } from "./IRequestOptions.js";
import { normalizeDeprecatedRequestOptions } from "./normalize-deprecated-request-options.js";

/**
* Helper for methods with lots of first order request options to pass through as request parameters.
* Appends selected custom option keys into `params` while preserving request options.
*
* @param customOptions Endpoint-specific options supplied by the caller.
* @param keys Keys from `customOptions` that should be appended to `params`.
* @param baseOptions Request defaults merged before `customOptions`.
* @return IRequestOptions object with custom options appended to `params`, and all other custom options removed that are not part of IRequestOptions.
*/
export function appendCustomParams<T extends IRequestOptions>(
customOptions: T,
Expand Down Expand Up @@ -50,10 +56,13 @@ export function appendCustomParams<T extends IRequestOptions>(
}, options.params);

// now remove all properties in options that don't exist in IRequestOptions
return requestOptionsKeys.reduce((value, key) => {
const requestOptions = requestOptionsKeys.reduce((value, key) => {
if ((options as any)[key]) {
(value as any)[key] = (options as any)[key];
}
return value;
}, {} as IRequestOptions);

// Normalize legacy top-level options into requestFlags/fetchOptions.
return normalizeDeprecatedRequestOptions(requestOptions);
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function normalizeDeprecatedRequestOptions(
};

const normalizedOptions: IRequestOptions = {
// some packages extend IRequestOptions with additional properties that we want to preserve, so we spread the original requestOptions here and then override the known deprecated options with their new equivalents
// some packages extend IRequestOptions with additional properties that we want to preserve, so we spread the extended requestOptions here, then override the known deprecated options with their new equivalents
// then delete the deprecated options at the end to avoid duplication
...(requestOptions as any),
params: requestOptions.params,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { appendCustomParams } from "../../src/index.js";
import { describe, test, expect } from "vitest";

describe("appendCustomParams", () => {
test("merges custom options and base options, handles all value types, and omits invalid keys", () => {
test("merges custom options and base options, handles all value types, omits invalid keys, and normalizes legacy request options", () => {
// this test should cover:
// - omit keys not in keys
// - deconstruct and merge customOptions into params
Expand Down Expand Up @@ -50,27 +50,25 @@ describe("appendCustomParams", () => {
a: 1
});

// baseOptions keys should be present
expect(result.httpMethod).toBe("POST");
expect(result.credentials).toBe("include");
// baseOptions keys should be normalized into fetchOptions
expect(result.fetchOptions).toEqual({
method: "POST",
credentials: "include",
headers: {}
});

// legacy keys should be normalized and removed
expect(result.httpMethod).toBeUndefined();
expect(result.credentials).toBeUndefined();

// result should only retain keys listed in requestOptionsKeys
Object.keys(result).forEach((key) => {
expect([
"params",
"httpMethod",
"rawResponse",
"authentication",
"requestFlags",
"fetchOptions",
"hideToken",
"portal",
"credentials",
"maxUrlLength",
"headers",
"signal",
"suppressWarnings",
"request"
"portal"
]).toContain(key);
});
});
Expand Down