Skip to content

scoped template and ability to pass available components #16

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidmeirlevy
Copy link

@davidmeirlevy davidmeirlevy commented May 24, 2023

I thought it can be safer if I make the template as scoped a possible.
probably there's still work here, but the idea is to pass the components it should be able to compile, and use only the props given in :template-props={}.

@mattelen what do you say?

@mattelen mattelen self-requested a review May 24, 2023 15:12
const methodsFromProps = buildFromProps(parent, methodKeys);
const finalProps = merge([
passthrough.$data,
passthrough.$props,
methodsFromProps,
this.templateProps,
]);
const components = this.scoped ? this.templateComponents : {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

components is already defined earlier, so my compiler to bugging out on this. Can you tweak this?

type: Object,
default: () => ({}),
},
scoped: Boolean,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we add a default to this? I'm thinking a default of false, then it's backward compatible.

@mattelen
Copy link
Owner

Thanks @davidmeirlevy for spending time on this! I get where you are coming from and I think this is a good idea. I've made some comments on the file - once you've fixed those issues, I'll test the code itself. Thanks

@davidmeirlevy
Copy link
Author

hi!

don't think I forgot this fix :)
I'm already using your library in my project so I have a lot of motivation to do it.

I have some other bugs to fix in my app, and I'll handle this PR too.

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

Successfully merging this pull request may close these issues.

2 participants