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

defaultProps: default function type is lost #847

Closed
1 of 11 tasks
kingzez opened this issue Oct 26, 2022 · 5 comments · Fixed by #896
Closed
1 of 11 tasks

defaultProps: default function type is lost #847

kingzez opened this issue Oct 26, 2022 · 5 comments · Fixed by #896
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@kingzez
Copy link
Collaborator

kingzez commented Oct 26, 2022

I am interested in helping provide a fix!

Yes

Which generators are impacted?

  • All
  • Angular
  • HTML
  • Qwik
  • React
  • React-Native
  • Solid
  • Stencil
  • Svelte
  • Vue
  • Web components

Reproduction case

https://mitosis.builder.io/?outputTab=G4VwpkA%3D&code=JYWwDg9gTgLgBAbzgVwM4FMDKNroDQoYAi6AZgIbIA2MAClBGKnAL5ykMhwDkAAgEbJgVACbooAOmAQA9CGA5UwVNwDcAKHXAAdjHEUAxujgAhZDBzb6jZgnVw4SgF7oAXI5hQdAcw0ORyuT8VOgi7vwQECHk2n5wENoAwlTABgDW7gAUbnAxAJ4AlHAAvAB8cABuEMAiGiya6AAekLBwYhTU8KTI2gYw0tqm5paZYAxM7mYWCdZMRXYOaOgkHTSzqAA8U5brpZkLDo7ALu7cIKHAyCDcePaHAahBIWHs5FQYt4fxSSnpWUVlRAsAgyGRwHAiCDsYCNODyRTKOAGCDgYTGbSRMDgqEIeoOFgFDR3KDoGDIKCDTJ3BwbQTTbTUw4JZKpNLFBDZCroXQA8qZACEYxsEgeT1CcAA%2FHAhUwJMzfmlOdyYEV3D12jpQgU8YdSoyHNsEoyNjI6ZY9Q5Cep6kA%3D

Expected Behaviour

onClick default value should be () => {}, but vue got {}, react got lost.

import { useStore, useDefaultProps } from '@builder.io/mitosis';

interface ButtonProps {
  size: string;
  disabled: boolean;
  onClick: (e: any) => void;
}

export default function Button(props: ButtonProps) {
  useDefaultProps<ButtonProps>({
    size: 'medium',
    disabled: false,
    onClick: () => {}, // todo fix mitosis compile noop to {}
  });

  return (
    <button
      onClick={(event) => (!props.disabled ? props.onClick(event) : undefined)}
    >
      Button
    </button>
  );
}

vue

<template>
  <button @click="!disabled ? onClick($event) : undefined">Button</button>
</template>

<script>
export default {
  name: "button",

  props: { disabled: { default: false }, onClick: () => {} },
};
</script>

react

import * as React from "react";

interface ButtonProps {
  size: string;
  disabled: boolean;
  onClick: (e: any) => void;
}

export default function Button(props: ButtonProps) {
  return (
    <button
      onClick={(event) => (!props.disabled ? props.onClick(event) : undefined)}
    >
      Button
    </button>
  );
}

Button.defaultProps = { size: "medium", disabled: false, onClick: () => {} };

Actual Behaviour

vue

<template>
  <button @click="!disabled ? onClick($event) : undefined">Button</button>
</template>

<script>
export default {
  name: "button",

  props: { disabled: { default: false }, onClick: {} },
};
</script>

react

import * as React from "react";

interface ButtonProps {
  size: string;
  disabled: boolean;
  onClick: (e: any) => void;
}

export default function Button(props: ButtonProps) {
  return (
    <button
      onClick={(event) => (!props.disabled ? props.onClick(event) : undefined)}
    >
      Button
    </button>
  );
}

Button.defaultProps = { size: "medium", disabled: false };

Additional Information

No response

@kingzez kingzez added the bug Something isn't working label Oct 26, 2022
@kingzez
Copy link
Collaborator Author

kingzez commented Oct 26, 2022

In function parseJsx, babel output no defaultProps.onClick

{
  metadata: {},
  options: {
    assumptions: {},
    configFile: false,
    babelrc: false,
    comments: false,
    targets: {},
    cloneInputAst: true,
    browserslistConfigFile: false,
    passPerPreset: false,
    envName: 'development',
    rootMode: 'root',
    plugins: [ [Plugin], [Plugin], [Plugin] ],
    presets: [],
    parserOpts: {
      sourceType: 'module',
      sourceFileName: undefined,
      plugins: [Array]
    },
    generatorOpts: {
      filename: undefined,
      auxiliaryCommentBefore: undefined,
      auxiliaryCommentAfter: undefined,
      retainLines: undefined,
      comments: false,
      shouldPrintComment: undefined,
      compact: 'auto',
      minified: undefined,
      sourceMaps: false,
      sourceRoot: undefined,
      sourceFileName: 'unknown'
    }
  },
  ast: null,
  code: '({\n' +
    '  "@type": "@builder.io/mitosis/component",\n' +
    '  "imports": [],\n' +
    '  "exports": {},\n' +
    '  "inputs": [],\n' +
    '  "meta": {},\n' +
    '  "refs": {},\n' +
    '  "state": {},\n' +
    '  "children": [{\n' +
    '    "@type": "@builder.io/mitosis/node",\n' +
    '    "name": "button",\n' +
    '    "meta": {},\n' +
    '    "scope": {},\n' +
    '    "properties": {},\n' +
    '    "bindings": {\n' +
    '      "onClick": {\n' +
    '        "code": "!props.disabled ? props.onClick(event) : undefined",\n' +
    '        "arguments": ["event"]\n' +
    '      }\n' +
    '    },\n' +
    '    "children": [{\n' +
    '      "@type": "@builder.io/mitosis/node",\n' +
    '      "name": "div",\n' +
    '      "meta": {},\n' +
    '      "scope": {},\n' +
    '      "properties": {\n' +
    '        "_text": "\\n      Button\\n    "\n' +
    '      },\n' +
    '      "bindings": {},\n' +
    '      "children": []\n' +
    '    }]\n' +
    '  }],\n' +
    '  "hooks": {},\n' +
    '  "context": {\n' +
    '    "get": {},\n' +
    '    "set": {}\n' +
    '  },\n' +
    '  "name": "Button",\n' +
    '  "subComponents": [],\n' +
    '  "types": ["interface ButtonProps {\\n  size: string;\\n  disabled: boolean;\\n  onClick: (e: any) => void;\\n}"],\n' +
    '  "defaultProps": {\n' +
    '    "size": "medium",\n' +
    '    "disabled": false\n' +
    '  },\n' +
    '  "propsTypeRef": "ButtonProps"\n' +
    '});',
  map: null,
  sourceType: 'script'
}

@kingzez
Copy link
Collaborator Author

kingzez commented Oct 26, 2022

The positioning problem is probably in the function parseDefaultPropsHook,Object type is not work either.

And json5.stringify also filter out function in core/generators/*

Am I right ?

@samijaber
Copy link
Contributor

Correct, we are using JSON.stringify which does not work for functions. We'll need to add support for functions.

I will provide some more details soon on how this can be accomplished

@samijaber
Copy link
Contributor

samijaber commented Nov 8, 2022

Steps to handle functions in defaultProps:

Update defaultProps type to MitosisState

1- Update the defaultProps type from JSONObject to MitosisState: https://github.com/BuilderIO/mitosis/blob/a762948e6014450d5b8b80c961b771df4568f847/packages/core/src/types/mitosis-component.ts#L123`

2- Update this part to match the MitosisState schema:

component.defaultProps = {
...(component.defaultProps ?? {}),
[i.key?.name]: i.value.value,
};

component.defaultProps = { 
   ...(component.defaultProps ?? {}), 
   [i.key?.name]: { type: 'property', code: `i.value.value` }, 
 }; 

3- Now update all uses of defaultProps in generators to use the code property. TypeScript will help point out where you need to do that.

Store functions by generating string

1- update the parseDefaultPropsHook function to process the code

const value = i.value.value;
if (types.isFunctionExpression(value) || types.isArrowFunctionExpression(value)) {
  const code = generate(firstArg.body)
    .code.trim()
    // Remove arbitrary block wrapping if any
    // AKA
    //  { console.log('hi') } -> console.log('hi')
    .replace(/^{/, '')
    .replace(/}$/, '');
  return { type: 'method', code }
} else {
  return { type: 'property', code: value }
}

2- I think this should work out of the box for simple cases. It will not yet handle props. and state. references inside of it,, but we can leave that as a follow-up as it will require a bunch more work.

@samijaber samijaber added the good first issue Good for newcomers label Nov 8, 2022
@kingzez
Copy link
Collaborator Author

kingzez commented Nov 9, 2022

I follow up.

kingzez added a commit to kingzez/mitosis that referenced this issue Nov 11, 2022
samijaber pushed a commit that referenced this issue Nov 29, 2022
* fix(core): defaultProps function type

#847

* fix(core): svelte default props

* test(core): update snapshots

* test: update snapshots

* fix: remove manually adding }

* feat: remove useless trim

* refactor: use parseStateObjectToMitosisState parseDefaultProps

* fix: remove useless import
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

Successfully merging a pull request may close this issue.

2 participants