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.

Sign up for GitHub

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

Jump to bottom

[core-client] Prepare release for v1.3.3 out of a hotfix branch #18856

Merged
merged 5 commits into from
Dec 2, 2021
Merged

[core-client] Prepare release for v1.3.3 out of a hotfix branch #18856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b2f93df
Check is value is undefined in appendQueryParams (#18621)
sarangan12 Nov 11, 2021
bf4277d
Post release automated changes for core releases (#18358)
azure-sdk Oct 26, 2021
9d375d3
Remove the word experimental in readmes (#18773)
ramya-rao-a Nov 23, 2021
564ad48
Update Changelog to include query param check (#18851)
sarangan12 Nov 29, 2021
b04216a
fix reinstall native dependency (#18582)
praveenkuttappan Nov 9, 2021
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
5 changes: 3 additions & 2 deletions eng/pipelines/templates/steps/use-node-test-version.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ steps:
}
# Map from the symlink path to the target path (npm has issues installing into symlink dirs)
# Example: common/temp/node_modules/.pnpm/keytar@5.6.0/node_modules/keytar
$targetPath = (Get-Item $symlink).Target
$symlinkInfo = Get-Item $symlink
$targetPath = [IO.Path]::Combine($symlinkInfo.Parent, $symlinkInfo.Target)
Write-Host "Target of symlink : $($targetPath)"

# Need to run "npm install" at path containing "node_modules" folder
# Example: common/temp/node_modules/.pnpm/keytar@5.6.0
Expand All @@ -42,7 +44,6 @@ steps:
# pnpm v6 replaces '/' in package names with '+' to reduce nesting directory in virtual store so we need to
# change it back
$packageAtVersion = $packageAtVersion.Replace("+", "/")

# Check if package has org name. for e.g @azure/msal-node-enxtensions
# This returns either @azure or .pnpm( if no org is present)
$packageParentName = Split-path -Leaf (Split-Path -Parent -Resolve $packageInstallPath)
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-client-lro-rest/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Azure Rest Core LRO library for JavaScript (Experimental)
# Azure Rest Core LRO library for JavaScript

This library is primarily intended to be used in code generated by [AutoRest](https://github.com/Azure/Autorest) and [`autorest.typescript`](https://github.com/Azure/autorest.typescript). Specifically for rest level clients, as a helper to handle long running operations. This package implements support for Autorest `x-ms-long-running-operation` specification.

Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-client-paging-rest/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Azure Rest Core Paging library for JavaScript (Experimental)
# Azure Rest Core Paging library for JavaScript

This library is primarily intended to be used in code generated by [AutoRest](https://github.com/Azure/Autorest) and [`autorest.typescript`](https://github.com/Azure/autorest.typescript). Specifically for rest level clients, as a helper to handle Pageable operations. This package implements support for Autorest `x-ms-pageable` specification.

Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-client-rest/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Azure Rest Core client library for JavaScript (Experimental)
# Azure Rest Core client library for JavaScript

This library is primarily intended to be used in code generated by [AutoRest](https://github.com/Azure/Autorest) and [`autorest.typescript`](https://github.com/Azure/autorest.typescript). Specifically for rest level clients

Expand Down
6 changes: 6 additions & 0 deletions sdk/core/core-client/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Release History

## 1.3.3 (2021-12-02)

### Bugs Fixed

- Added a check to handle undefined value during the parsing of query parameters. Please refer to [PR #18621](https://github.com/Azure/azure-sdk-for-js/pull/18621) for further details.

## 1.3.2 (2021-10-25)

### Bugs Fixed
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-client/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Azure Core Service client library for JavaScript (Experimental)
# Azure Core Service client library for JavaScript

This library is primarily intended to be used in code generated by [AutoRest](https://github.com/Azure/Autorest) and [`autorest.typescript`](https://github.com/Azure/autorest.typescript).

Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-client/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@azure/core-client",
"version": "1.3.2",
"version": "1.3.3",
"description": "Core library for interfacing with AutoRest generated code",
"sdk-type": "client",
"main": "dist/index.js",
Expand Down
11 changes: 8 additions & 3 deletions sdk/core/core-client/src/urlHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,11 @@ function calculateQueryParameters(
};
}

function simpleParseQueryParams(queryString: string): Map<string, string | string[]> {
const result: Map<string, string | string[]> = new Map<string, string | string[]>();
function simpleParseQueryParams(queryString: string): Map<string, string | string[] | undefined> {
const result: Map<string, string | string[] | undefined> = new Map<
string,
string | string[] | undefined
>();
if (!queryString || queryString[0] !== "?") {
return result;
}
Expand Down Expand Up @@ -288,11 +291,13 @@ export function appendQueryParams(
for (const [name, value] of combinedParams) {
if (typeof value === "string") {
searchPieces.push(`${name}=${value}`);
} else {
} else if (Array.isArray(value)) {
// QUIRK: If we get an array of values, include multiple key/value pairs
for (const subValue of value) {
searchPieces.push(`${name}=${subValue}`);
}
} else {
searchPieces.push(`${name}=${value}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, is this designed to handle undefined? Wouldn’t it add undefined as the string value here?

Copy link
Member

Choose a reason for hiding this comment

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

yes, it would add undefined as the literal value

Copy link
Contributor

Choose a reason for hiding this comment

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

is that intentional? if so, disregard.

Copy link
Member

Choose a reason for hiding this comment

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

some context: #18621 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

@sarangan12 perhaps consider creating a test case for this behavior to document it.

Copy link
Contributor

@sadasant sadasant Nov 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Wait, shouldn’t we skip it? would it work too or would it fail? meaning, to remove things like C=undefined. The link in context indicates that C=undefined works, but it does not indicate that removing that query property would fail.

Copy link
Member

Choose a reason for hiding this comment

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

I'm a little torn, since silently removing the query params seems like a slightly larger jump than filling in a bogus value. @sarangan12 did your investigation yield if the presence of this parameter mattered at all?

Copy link
Member

Choose a reason for hiding this comment

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

@xirzec Yes. Removing the value will cause the error if the value is not an optional value.

}
}

Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-rest-pipeline/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Azure Core HTTP client library for JavaScript (Experimental)
# Azure Core HTTP client library for JavaScript

This is the core HTTP pipeline for Azure SDK JavaScript libraries which work in the browser and Node.js. This library is primarily intended to be used in code generated by [AutoRest](https://github.com/Azure/Autorest) and [`autorest.typescript`](https://github.com/Azure/autorest.typescript).

Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-xml/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Azure Core XML client library for JavaScript (Experimental)
# Azure Core XML client library for JavaScript

This library is primarily intended to be used in code generated by [AutoRest](https://github.com/Azure/Autorest) and [`autorest.typescript`](https://github.com/Azure/autorest.typescript) for APIs that require parsing XML payloads.

Expand Down