Skip to content

remove String extension in LazyJsonString #1468

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

Merged
merged 3 commits into from
Dec 9, 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
5 changes: 5 additions & 0 deletions .changeset/warm-dragons-wash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@smithy/smithy-client": minor
---

remove String extension in LazyJsonString
14 changes: 10 additions & 4 deletions packages/smithy-client/src/lazy-json.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { describe, expect, test as it } from "vitest";

import { LazyJsonString } from "./lazy-json";

describe("LazyJsonString", () => {
it("should has string methods", () => {
it("should have string methods", () => {
const jsonValue = new LazyJsonString('"foo"');
expect(jsonValue.length).toBe(5);
expect(jsonValue.toString()).toBe('"foo"');
Expand All @@ -22,17 +23,22 @@ describe("LazyJsonString", () => {

it("can instantiate from LazyJsonString class", () => {
const original = new LazyJsonString('"foo"');
const newOne = LazyJsonString.fromObject(original);
const newOne = LazyJsonString.from(original);
expect(newOne.toString()).toBe('"foo"');
});

it("can instantiate from String class", () => {
const jsonValue = LazyJsonString.fromObject(new String('"foo"'));
const jsonValue = LazyJsonString.from(new String('"foo"'));
expect(jsonValue.toString()).toBe('"foo"');
});

it("can instantiate from object", () => {
const jsonValue = LazyJsonString.fromObject({ foo: "bar" });
const jsonValue = LazyJsonString.from({ foo: "bar" });
expect(jsonValue.toString()).toBe('{"foo":"bar"}');
});

it("passes instanceof String check", () => {
const jsonValue = LazyJsonString.from({ foo: "bar" });
expect(jsonValue).toBeInstanceOf(String);
});
});
104 changes: 60 additions & 44 deletions packages/smithy-client/src/lazy-json.ts
Original file line number Diff line number Diff line change
@@ -1,57 +1,73 @@
/**
* Lazy String holder for JSON typed contents.
* @public
*
* A model field with this type means that you may provide a JavaScript
* object in lieu of a JSON string, and it will be serialized to JSON
* automatically before being sent in a request.
*
* For responses, you will receive a "LazyJsonString", which is a boxed String object
* with additional mixin methods.
* To get the string value, call `.toString()`, or to get the JSON object value,
* call `.deserializeJSON()` or parse it yourself.
*/

interface StringWrapper {
new (arg: any): String;
}
export type AutomaticJsonStringConversion = Parameters<typeof JSON.stringify>[0] | LazyJsonString;

/**
* Because of https://github.com/microsoft/tslib/issues/95,
* TS 'extends' shim doesn't support extending native types like String.
* So here we create StringWrapper that duplicate everything from String
* class including its prototype chain. So we can extend from here.
*
* @internal
*
*/
// @ts-ignore StringWrapper implementation is not a simple constructor
export const StringWrapper: StringWrapper = function () {
//@ts-ignore 'this' cannot be assigned to any, but Object.getPrototypeOf accepts any
const Class = Object.getPrototypeOf(this).constructor;
const Constructor = Function.bind.apply(String, [null as any, ...arguments]);
//@ts-ignore Call wrapped String constructor directly, don't bother typing it.
const instance = new Constructor();
Object.setPrototypeOf(instance, Class.prototype);
return instance as String;
};
StringWrapper.prototype = Object.create(String.prototype, {
constructor: {
value: StringWrapper,
enumerable: false,
writable: true,
configurable: true,
},
});
Object.setPrototypeOf(StringWrapper, String);
export interface LazyJsonString extends String {
new (s: string): typeof LazyJsonString;

/**
* @returns the JSON parsing of the string value.
*/
deserializeJSON(): any;

/**
* @returns the original string value rather than a JSON.stringified value.
*/
toJSON(): string;
}

/**
* @internal
*
* Extension of the native String class in the previous implementation
* has negative global performance impact on method dispatch for strings,
* and is generally discouraged.
*
* This current implementation may look strange, but is necessary to preserve the interface and
* behavior of extending the String class.
*/
export class LazyJsonString extends StringWrapper {
deserializeJSON(): any {
return JSON.parse(super.toString());
}
export function LazyJsonString(val: string): void {
const str = Object.assign(new String(val), {
deserializeJSON() {
return JSON.parse(String(val));
},

toJSON(): string {
return super.toString();
}
toString() {
return String(val);
},

static fromObject(object: any): LazyJsonString {
if (object instanceof LazyJsonString) {
return object;
} else if (object instanceof String || typeof object === "string") {
return new LazyJsonString(object);
}
return new LazyJsonString(JSON.stringify(object));
}
toJSON() {
return String(val);
},
});

return str as never;
}

LazyJsonString.from = (object: any): LazyJsonString => {
if (object && typeof object === "object" && (object instanceof LazyJsonString || "deserializeJSON" in object)) {
return object as any;
} else if (typeof object === "string" || Object.getPrototypeOf(object) === String.prototype) {
return LazyJsonString(String(object) as string) as any;
}
return LazyJsonString(JSON.stringify(object)) as any;
};

/**
* @deprecated use from.
*/
LazyJsonString.fromObject = LazyJsonString.from;
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,12 @@ public Symbol stringShape(StringShape shape) {
if (mediaTypeTrait.isPresent()) {
String mediaType = mediaTypeTrait.get().getValue();
if (CodegenUtils.isJsonMediaType(mediaType)) {
Symbol.Builder builder = createSymbolBuilder(shape, "__LazyJsonString | string");
return addSmithyUseImport(builder, "LazyJsonString", "__LazyJsonString").build();
Symbol.Builder builder = createSymbolBuilder(shape, "__AutomaticJsonStringConversion | string");
return addSmithyUseImport(
builder,
"AutomaticJsonStringConversion",
"__AutomaticJsonStringConversion"
).build();
} else {
LOGGER.warning(() -> "Found unsupported mediatype " + mediaType + " on String shape: " + shape);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public static String getStringInputParam(GenerationContext context, Shape shape,
if (CodegenUtils.isJsonMediaType(mediaType)) {
TypeScriptWriter writer = context.getWriter();
writer.addImport("LazyJsonString", "__LazyJsonString", TypeScriptDependency.AWS_SMITHY_CLIENT);
return "__LazyJsonString.fromObject(" + dataSource + ")";
return "__LazyJsonString.from(" + dataSource + ")";
} else {
LOGGER.warning(() -> "Found unsupported mediatype " + mediaType + " on String shape: " + shape);
}
Expand Down Expand Up @@ -210,7 +210,7 @@ public static String getStringOutputParam(GenerationContext context,
if (CodegenUtils.isJsonMediaType(mediaType)) {
TypeScriptWriter writer = context.getWriter();
writer.addImport("LazyJsonString", "__LazyJsonString", TypeScriptDependency.AWS_SMITHY_CLIENT);
return "new __LazyJsonString(" + dataSource + ")";
return "__LazyJsonString.from(" + dataSource + ")";
} else {
LOGGER.warning(() -> "Found unsupported mediatype " + mediaType + " on String shape: " + shape);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public void usesLazyJsonStringForJsonMediaType() {
SymbolProvider provider = new SymbolVisitor(model, settings);
Symbol memberSymbol = provider.toSymbol(member);

assertThat(memberSymbol.getName(), equalTo("__LazyJsonString | string"));
assertThat(memberSymbol.getName(), equalTo("__AutomaticJsonStringConversion | string"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public static Collection<Object[]> validMemberTargetTypes() {
{StringShape.builder().id(id).build(), "__expectString(" + DATA_SOURCE + ")", source},
{
StringShape.builder().id(id).addTrait(new MediaTypeTrait("foo+json")).build(),
"new __LazyJsonString(" + DATA_SOURCE + ")",
"__LazyJsonString.from(" + DATA_SOURCE + ")",
source
},
{BlobShape.builder().id(id).build(), "context.base64Decoder(" + DATA_SOURCE + ")", source},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public static Collection<Object[]> validMemberTargetTypes() {
{StringShape.builder().id(id).build(), DATA_SOURCE},
{
StringShape.builder().id(id).addTrait(new MediaTypeTrait("foo+json")).build(),
"__LazyJsonString.fromObject(" + DATA_SOURCE + ")"
"__LazyJsonString.from(" + DATA_SOURCE + ")"
},
{BlobShape.builder().id(id).build(), "context.base64Encoder(" + DATA_SOURCE + ")"},
{DocumentShape.builder().id(id).build(), delegate},
Expand Down
Loading