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

poc: common JSX components #5952

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

poc: common JSX components #5952

wants to merge 4 commits into from

Conversation

aymeric-giraudet
Copy link
Member

@aymeric-giraudet aymeric-giraudet commented Dec 5, 2023

Only tested with React and Vue 2 for now, the examples still work well.

One problem for now with Vue 2 is that we can't use DefaultHitComponent, as you can't call h with a function component. Maybe Vue 3 will work. Otherwise we can call it as a function as it doesn't have state so it may be fine not to have it as a React/Preact fiber.

Next is to try with Preact (expecting TS errors) and Vue 3.

Also has to keep in mind : in React we also pass div props, we need to find a way to properly type it for the user, without relying on React types in instantsearch-jsx.

Update 1 : Vue3 works out of the box, moved Vue2 compat code in its own file and now the same code works for both versions, even DefaultHitComponent works ! (save for slots which are still under a condition)

Update 2 : For Preact surprisingly types worked easily, however with the way we do templating, we need to have an additional itemComponent prop which contains the li itself :/

Copy link

codesandbox-ci bot commented Dec 5, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e872fb0:

Sandbox Source
InstantSearch.js Configuration
react-instantsearch-app Configuration
example-react-instantsearch-default-theme Configuration
example-react-instantsearch-next-app-dir-example Configuration
example-vue-instantsearch-default-theme Configuration

Comment on lines 8 to 11
const augmentH = (baseH) => (tag, propsWithClassName, children) => {
const { className, ...props } = propsWithClassName;
return baseH(tag, Object.assign(props, { class: className }), [children]);
};
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to map className to class and children has to be an array for Vue's h function (they don't have fragments)

Copy link
Contributor

Choose a reason for hiding this comment

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

vue 2 and 3 are quite different, we have renderCompat, but I wonder if we should just have separate files for vue 2 and vue 3 once we simplify the rendering?


const h = augmentH(baseH);

return createHits({ createElement: h })({
Copy link
Member Author

Choose a reason for hiding this comment

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

Call to createHits could be externalized so that we don't call it every render

Copy link
Contributor

Choose a reason for hiding this comment

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

how can it be externalised? if I remember it correctly in vue 2 you only have access to h in render's arguments

Copy link
Member Author

Choose a reason for hiding this comment

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

Considering Vue 2 will soon reach EOL, we might want to only support Vue 3 so this wouldn't be a problem.
Else we can make 1 file per version, we'll see what's best !

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.

None yet

2 participants