Skip to content
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

Comments cause crash #250

Open
samijaber opened this issue Mar 28, 2022 · 6 comments
Open

Comments cause crash #250

samijaber opened this issue Mar 28, 2022 · 6 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@samijaber
Copy link
Contributor

samijaber commented Mar 28, 2022

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

@samijaber samijaber added the bug Something isn't working label Mar 28, 2022
@ocombe
Copy link

ocombe commented Apr 1, 2022

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 samijaber changed the title Comments within useState block cause crash Comments cause crash Apr 1, 2022
@samijaber samijaber added the good first issue Good for newcomers label Jun 30, 2022
@samijaber
Copy link
Contributor Author

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
Copy link

I'm working on this 👍🏾

@samijaber
Copy link
Contributor Author

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:

path.replaceWith(jsonToAst(componentFunctionToJson(node, context)));

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

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
Copy link

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
Copy link

magne4000 commented May 29, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants