remove String extension in LazyJsonString (#1468)

* remove String extension in LazyJsonString

* uniform usage of LazyJsonString

* preserve LazyJsonString interface

Author: kuhe

.changeset/warm-dragons-wash.md

@@ -0,0 +1,5 @@
+---
+"@smithy/smithy-client": minor
+---
+
+remove String extension in LazyJsonString

packages/smithy-client/src/lazy-json.spec.ts

@@ -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"');
@@ -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);
+  });
 });

packages/smithy-client/src/lazy-json.ts

@@ -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;

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/SymbolVisitor.java

@@ -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);
             }

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java

@@ -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);
             }
@@ -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);
             }

smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/SymbolProviderTest.java

@@ -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

smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberDeserVisitorTest.java

@@ -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},

smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/integration/DocumentMemberSerVisitorTest.java

@@ -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},