Skip to content

Commit

Permalink
remove String extension in LazyJsonString (#1468)
Browse files Browse the repository at this point in the history
* remove String extension in LazyJsonString

* uniform usage of LazyJsonString

* preserve LazyJsonString interface
  • Loading branch information
kuhe authored Dec 9, 2024
1 parent 7c8345f commit 70275bd
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 55 deletions.
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

0 comments on commit 70275bd

Please sign in to comment.