fix: react render config #45

dpilch · 2021-09-02T20:41:09Z

The major changes:

Made a FrameworkRenderConfig and ReactRenderConfig because we needed to pass these configurations into the renderer and not the output manager
Use the transpileModule from typescript to set certain targets and modules
- I haven’t found a way to use the transpileModule and keep typescript.
  - For TSX I set it to not pass through transpileModule to preserve the syntax
  - For TS I have not found a was to transpile the React code, but still preserve the TS syntax. I think we should push this to another change.

Example:
amplify component:

{
  "componentId": "1234-5678-9010",
  "componentType": "Text",
  "name": "CustomText",
  "properties": {
    "color": {
      "value": "#ff0000"
    },
    "width": {
      "value": "20px"
    },
    "value": {
      "value": "Text Value"
    }
  }
}

With config:

const renderConfig: ReactRenderConfig = {
  module: ModuleEnum.CommonJS,
  script: ScriptEnum.js,
}

Will produce:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
/* eslint-disable */
const react_1 = require("react");
const ui_react_1 = require("@aws-amplify/ui-react");
function CustomButton(props) {
  return react_1.default.createElement(
    ui_react_1.Button,
    Object.assign(
      {
        color: "#ff0000",
        width: 20,
        label: buttonUser.username || "hspain@gmail.com",
        labelWidth: width || "50px",
      },
      props,
      getOverrideProps(props.overrides, "Button")
    )
  );
}
exports.default = CustomButton;

TODO in future change:

Figure out TS script kind
Raise warnings on invalid render config combinations. i.e. CommonJS + JSX

dpilch · 2021-09-02T20:45:37Z

packages/studio-ui-codegen-react/lib/index.ts

-export * from "./react-studio-template-renderer";
-export * from "./react-output-config";
-
-export class ReactOutputManager extends FrameworkOutputManager<string> {


This code was duplicated across this file and packages/studio-ui-codegen-react/lib/react-output-manager.ts.

cwoolum

Just a little bit of feedback! Looks very good overall though!

cwoolum · 2021-09-02T22:30:37Z

packages/studio-ui-codegen-react/lib/react-studio-template-renderer.ts


-  constructor(component: StudioComponent) {
-    super(component, new ReactOutputManager());
+  constructor(component: StudioComponent, renderConfig: ReactRenderConfig) {


If you use protected renderConfig: ReactRenderConfig, you won't need to cast this later on.

Thanks, will fix

cwoolum · 2021-09-02T22:30:53Z

packages/studio-ui-codegen-react/lib/react-studio-template-renderer.ts

@@ -88,11 +99,11 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer

    const result = printer.printNode(EmitHint.Unspecified, wrappedFunction, file);

+    const { script } = this.renderConfig as ReactRenderConfig;


See comment above about adding protected.

cwoolum · 2021-09-02T22:31:39Z

packages/studio-ui-codegen-react/lib/react-studio-template-renderer.ts

@@ -132,16 +143,33 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer
    const result = printer.printNode(EmitHint.Unspecified, wrappedFunction, file);
    componentText += result;

-    console.log(componentText);
+    const { script } = this.renderConfig as ReactRenderConfig;


See comment above about adding protected.

cwoolum · 2021-09-02T22:32:19Z

packages/studio-ui-codegen-react/lib/react-studio-template-renderer.ts

+    const file = createSourceFile(
+      this.fileName,
+      '',
+      target as unknown as ScriptTarget,


Can we coerce this rather than cast to unknown?

I was getting this error before. I'll look into it further to see if there is another fix.

Conversion of type 'TargetEnum | undefined' to type 'ScriptTarget' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first. Type 'TargetEnum.ES6' is not comparable to type 'ScriptTarget'.

I tried creating a subset of the TypeScript enum, but it seems the only way to do that is change it to a type union.

import { ScriptTarget } from 'typescript'; export type TargetEnum = Exclude<ScriptTarget, ScriptTarget.ES3>;

TargetEnum.ES6 // 'TargetEnum' only refers to a type, but is being used as a value here.

Another option would be to use the ScriptTarget enum directly, but then a user could input any of the available option supported by TypeScript. https://github.com/microsoft/TypeScript/blob/9d443b76aac0832d7f3c890441264d39307fe31a/lib/protocol.d.ts#L2753-L2765

We technically can support any ScriptTarget, right? Maybe it would be best to do that. Otherwise, it might be good to create a helper function that maps the two.

cwoolum · 2021-09-02T22:32:24Z

packages/studio-ui-codegen-react/lib/react-studio-template-renderer.ts

+      '',
+      target as unknown as ScriptTarget,
+      false,
+      script as unknown as ScriptKind,


Can we coerce this rather than cast to unknown?

packages/studio-ui-codegen/lib/studio-template-renderer.ts

cwoolum

LGTM

dpilch requested a review from frimfram September 2, 2021 20:41

dpilch changed the title ~~fix: react render config~~ [WIP] fix: react render config Sep 2, 2021

dpilch commented Sep 2, 2021

View reviewed changes

dpilch force-pushed the fix-react-config branch from 1dc119d to 66efa4c Compare September 2, 2021 21:02

frimfram requested a review from cwoolum September 2, 2021 22:13

cwoolum requested changes Sep 2, 2021

View reviewed changes

dpilch force-pushed the fix-react-config branch 2 times, most recently from b29c3d7 to d9c7c39 Compare September 7, 2021 16:29

dpilch changed the title ~~[WIP] fix: react render config~~ fix: react render config Sep 7, 2021

dpilch marked this pull request as ready for review September 7, 2021 16:31

dpilch requested a review from johnpc September 7, 2021 17:20

fix: react render config

c83fa7a

dpilch force-pushed the fix-react-config branch from d9c7c39 to c83fa7a Compare September 8, 2021 20:15

cwoolum approved these changes Sep 8, 2021

View reviewed changes

dpilch merged commit de74357 into main Sep 8, 2021

dpilch deleted the fix-react-config branch September 8, 2021 20:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: react render config #45

fix: react render config #45

dpilch commented Sep 2, 2021 •

edited

Loading

dpilch Sep 2, 2021

cwoolum left a comment

cwoolum Sep 2, 2021

dpilch Sep 3, 2021

cwoolum Sep 2, 2021

cwoolum Sep 2, 2021

cwoolum Sep 2, 2021

dpilch Sep 3, 2021 •

edited

Loading

dpilch Sep 3, 2021

cwoolum Sep 3, 2021

cwoolum Sep 2, 2021

cwoolum left a comment

		@@ -88,11 +99,11 @@ export abstract class ReactStudioTemplateRenderer extends StudioTemplateRenderer

		const result = printer.printNode(EmitHint.Unspecified, wrappedFunction, file);

		const { script } = this.renderConfig as ReactRenderConfig;

fix: react render config #45

fix: react render config #45

Conversation

dpilch commented Sep 2, 2021 • edited Loading

Choose a reason for hiding this comment

cwoolum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpilch Sep 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwoolum left a comment

Choose a reason for hiding this comment

dpilch commented Sep 2, 2021 •

edited

Loading

dpilch Sep 3, 2021 •

edited

Loading