BuilderIO / mitosis

Write components once, run everywhere. Compiles to React, Vue, Qwik, Solid, Angular, Svelte, and more.
https://mitosis.builder.io
MIT License
12.15k stars 533 forks source link

Comments cause crash #250

Open samijaber opened 2 years ago

samijaber commented 2 years ago

There are a bunch of different places where comments cause the compiler to crash, because we try to JSON5 parse the given block (and comments aren't JSON5 parse-able).

Describe the bug comments right above function declarations in useState cause endless loop/crash in Mitosis

To Reproduce

image
import { useState } from "@builder.io/mitosis";

export default function MyComponent(props) {
  const state = useState({
    // a comment
    name: 'steve',

    // another comment
    handleFn() {}
  })

  return (
    <div>
      <input
        css={{
          color: "red",
        }}
        value={state.name}
        onChange={(event) => state.name = (event.target.value)}
      />
      Hello
      {name}! I can run in React, Vue, Solid, or Liquid!
    </div>
  );
}

If possible, a link to a https://mitosis.builder.io/ fiddle containing the bug: Fiddle causes infinite loop, so I put a code block here instead.

Expected behavior Expect comments to be ignored/stripped before any Mitosis logic runs, and not cause any issues

Additional context The comment above a constant property works fine, but one above a function declaration isn't

ocombe commented 2 years ago

I had the same issue with a comment at the beginning of the file:

// hey
import { Show } from '@builder.io/mitosis';

export interface ButtonProps {
  attributes?: any;
  text?: string;
  link?: string;
  openLinkInNewTab?: boolean;
}

export default function Button(props: ButtonProps) {
  return (
    <>
      <Show
        when={props.link}
        else={<span {...props.attributes}>{props.text}</span>}
      >
        <a
          {...props.attributes}
          role="button"
          href={props.link}
          target={props.openLinkInNewTab ? '_blank' : undefined}
        >
          {props.text}
        </a>
      </Show>
    </>
  );
}

will generate the following error with the compiler:

Could not JSON5 parse object:
 // hey
({
  "@type": "@builder.io/mitosis/component",
  "imports": [],
  "meta": {},
  "state": {},
  "children": [{
    "@type": "@builder.io/mitosis/node",
    "name": "Fragment",
    "meta": {},
    "properties": {},
    "bindings": {},
    "children": [{
      "@type": "@builder.io/mitosis/node",
      "name": "div",
      "meta": {},
      "properties": {
        "_text": "\n      "
      },
      "bindings": {},
      "children": []
    }, {
      "@type": "@builder.io/mitosis/node",
      "name": "Show",
      "meta": {
        "else": {
          "@type": "@builder.io/mitosis/node",
          "name": "span",
          "meta": {},
          "properties": {},
          "bindings": {
            "_spread": "props.attributes"
          },
          "children": [{
            "@type": "@builder.io/mitosis/node",
            "name": "div",
            "meta": {},
            "properties": {},
            "bindings": {
              "_text": "props.text"
            },
            "children": []
          }]
        }
      },
      "properties": {},
      "bindings": {
        "when": "props.link"
      },
      "children": [{
        "@type": "@builder.io/mitosis/node",
        "name": "div",
        "meta": {},
        "properties": {
          "_text": "\n        "
        },
        "bindings": {},
        "children": []
      }, {
        "@type": "@builder.io/mitosis/node",
        "name": "a",
        "meta": {},
        "properties": {
          "role": "button"
        },
        "bindings": {
          "_spread": "props.attributes",
          "href": "props.link",
          "target": "props.openLinkInNewTab ? '_blank' : undefined"
        },
        "children": [{
          "@type": "@builder.io/mitosis/node",
          "name": "div",
          "meta": {},
          "properties": {
            "_text": "\n          "
          },
          "bindings": {},
          "children": []
        }, {
          "@type": "@builder.io/mitosis/node",
          "name": "div",
          "meta": {},
          "properties": {},
          "bindings": {
            "_text": "props.text"
          },
          "children": []
        }, {
          "@type": "@builder.io/mitosis/node",
          "name": "div",
          "meta": {},
          "properties": {
            "_text": "\n        "
          },
          "bindings": {},
          "children": []
        }]
      }, {
        "@type": "@builder.io/mitosis/node",
        "name": "div",
        "meta": {},
        "properties": {
          "_text": "\n      "
        },
        "bindings": {},
        "children": []
      }]
    }, {
      "@type": "@builder.io/mitosis/node",
      "name": "div",
      "meta": {},
      "properties": {
        "_text": "\n    "
      },
      "bindings": {},
      "children": []
    }]
  }],
  "hooks": {},
  "context": {
    "get": {},
    "set": {}
  },
  "name": "Button",
  "subComponents": []
}

Removing the comment fixes the issue

samijaber commented 2 years ago

Similarly, comments within JSX cause issues: see

They end up making it to the final version of the code. The parser thinks that div //@tsignore is the name of a component.

As a first step, we might want to trim all //... comments from the end of every code block.

SamiKamal commented 9 months ago

I'm working on this ๐Ÿ‘๐Ÿพ

samijaber commented 7 months ago

I think one of the main reasons we have these issues is that the JSX parser works by updating the AST and replacing it with the appropriate JSON:

https://github.com/BuilderIO/mitosis/blob/8980fd5d4130ffa719d42469677d1667bce0953a/packages/core/src/parsers/jsx/jsx.ts#L148

Instead, we could try storing the output of jsonToAst(componentFunctionToJson(node, context)) into a new variable and using it here instead of output.code:

https://github.com/BuilderIO/mitosis/blob/8980fd5d4130ffa719d42469677d1667bce0953a/packages/core/src/parsers/jsx/jsx.ts#L187

Let me know how that plays out. I think it should fix the issues around top-level comments at least, but not sure about the comments in JSX ๐Ÿ˜…

DutchJelly commented 6 months ago

A simple solution would be to just remove the comments. Is that an option? Otherwise, I think encoding the comments in json5 could be an option.

magne4000 commented 2 months ago

Keeping the comments and adding them in the JSON would actually be helpful in some cases. Comments could be used by plugins to customize compilation. They could for instance appear in meta.comments.

EDIT: in fact, a more fine grained "positioning" of the comments inside the JSON would be great (i.e. keep the relation between a Node and the comments above or after it). That could help reposition them in compiled code at the "same" place, thus documenting the code.