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

Props with Vue3 and Typescript #86

Open
Lalaluka opened this issue Aug 13, 2021 · 5 comments
Open

Props with Vue3 and Typescript #86

Lalaluka opened this issue Aug 13, 2021 · 5 comments

Comments

@Lalaluka
Copy link
Contributor

Hi,
I had a go on migrating some of our Vue2 Microfrontends to Vue3.
Also, I threw Typescript into the mix.

I'm not sure if it's an issue with the Package itself or just a missing bit of the documentation.

I encountered a problem with the usage of props. The SingleSpaOptsVue3 type doesn't have the props defined to typescript won't compile it. This is of course Typescript exclusive

const vueLifecycles = singleSpaVue({
  createApp,
  appOptions: {
    render() {
      return h(App, {
        // single-spa props are available on the "this" object. Forward them to your component as needed.
        // https://single-spa.js.org/docs/building-applications#lifecyle-props
        // if you uncomment these, remember to add matching prop definitions for them in your App.vue file.
        /*
        name: this.name,
        mountParcel: this.mountParcel,
        */
        singleSpa: this.singleSpa,
        //Property 'singleSpa' does not exist on type 'AppOptions | ((opts: SingleSpaOptsVue2 | SingleSpaOptsVue3, props: object) => Promise<AppOptions>)'.
        //Property 'singleSpa' does not exist on type '(opts: SingleSpaOptsVue2 | SingleSpaOptsVue3, props: object) => Promise<AppOptions>'.ts(2339)
      });
    },
  },
});

It can easily be fixed with an this alis (which is kind of ugly and talint doesn't like):

const vueLifecycles = singleSpaVue({
  createApp,
  appOptions: {
    render() {
      const thisalias: any = this;
      return h(App, {
        // single-spa props are available on the "this" object. Forward them to your component as needed.
        // https://single-spa.js.org/docs/building-applications#lifecyle-props
        // if you uncomment these, remember to add matching prop definitions for them in your App.vue file.
        /*
        name: this.name,
        mountParcel: this.mountParcel,
        */
        singleSpa: thisalias.singleSpa,
      });
    },
  },
});

Using appOptions as an async function works of course (with some types on the props).

const vueLifecycles = singleSpaVue({
  createApp,
  async appOptions(props: any) {
    return {
      render: h(App, {
        singleSpa: props.singleSpa,
      })
    }
  },
});

Maybe this should be described a bit better in the Docs? Or also improved in the generation? The example Comment suggesting to uncomment the Props is a bit wrong if Typescript is in place.

@joeldenning
Copy link
Member

This sounds like the typescript definitions need to be updated. Here's the file where they can be updated:

https://github.com/single-spa/single-spa-vue/blob/main/types/single-spa-vue.d.ts

The single-spa library has the default props defined at https://github.com/single-spa/single-spa/blob/f67d6fcd404597a246636e302f1d9e7b0f8cd9c6/typings/single-spa.d.ts#L12 - those should be imported into single-spa-vue's types and then used so that this has those props on them.

Do you have interest in contributing a pull request with the fix?

@Lalaluka
Copy link
Contributor Author

Yea im interested and will take a closer look the next few days.

Thanks for the initial ressources they look pretty helpfull!

@Lalaluka
Copy link
Contributor Author

HI @joeldenning
sorry for the late response.

I think the props are already handled in

type AppOptions = {
el?: string | HTMLElement;
data?: any;
[key: string]: any;
};
by the [key: string]: any; of course you could add the single-spa specific props there too (if wanted I have a PR ready).

But the real issue is the Union in line 15 onwards.

appOptions:
| AppOptions
| ((
opts: SingleSpaOptsVue2 | SingleSpaOptsVue3,
props: object
) => Promise<AppOptions>);
template?: string;

This leads understandably to the errors.
Tbh I would recommend updating https://github.com/single-spa/vue-cli-plugin-single-spa/blob/18e92aa559b489128edda8a5675f839e8022a267/template/src/main-vue-3.js#L12-L19 for Typescript configs updating the occurrences of this (from line 16-18) with a typecast like this: (<AppOptions>this).singleSpa.

I don't know if this is the preferred way. If yes I would be happy to contribute these changes.

tldr. Proposed Changes:

  • Add SingleSpa specific Props to single-spa-vue
  • Add typescript only typecast to vue3 Template

@joeldenning
Copy link
Member

I think the best solution would be to type the render function properly. Typecasting with (<AppOptions>this) is a workaround, but the full solution is to change the function signature.

The first thing to try would be to add the render function along with the this type to single-spa-vue:

type AppOptions = {
el?: string | HTMLElement;
data?: any;
[key: string]: any;
};

+ import { AppProps } from 'single-spa';
+
  type AppOptions = {
    el?: string | HTMLElement;
    data?: any;
+   render: (this: AppProps) => any;
    [key: string]: any;
  };

Since this is caller-dependent, I'm not sure if that will work. If not, you could make a similar change to vue-cli-plugin-single-spa. The this should be AppProps, not AppOptions, since it's the single-spa props that are put on the this, not the options.

@kosmos
Copy link

kosmos commented Dec 15, 2021

I have exactly the same issue. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants