Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

feat: add experimental composition APIs #1052

Closed
wants to merge 19 commits into from
Closed

Conversation

eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Sep 1, 2021

Summary

This PR adds experimental composition APIs.

For now, useConfigure, useRefinementList, useSearchBox, and useConnector are implemented.

In case of useRefinementList, I referred to François' PoC with React InstantSearch.

vue 3

<template>
  <div>
    <p>query: {{ searchBox.query }}</p>
  </div>
</template>

<script>
import { EXPERIMENTAL_useSearchBox } from 'vue-instantsearch/vue3/es';

export default {
  setup() {
    return { searchBox: EXPERIMENTAL_useSearchBox() };
  }
}

vue 2

yarn add @vue/composition-api
# or
npm install @vue/composition-api
// main.js
import Vue from 'vue';
import VueCompositionAPI from '@vue/composition-api';

Vue.use(VueCompositionAPI);
// Child.vue

<template>
  <div>
    <p>query: {{ searchBox.query }}</p>
  </div>
</template>

<script>
import { EXPERIMENTAL_useSearchBox } from 'vue-instantsearch/vue2/es/compositions';
// Import composition APIs from the path above.
// The rest still remain as `import xxx from 'vue-instantsearch'`.

export default {
  setup() {
    return { searchBox: EXPERIMENTAL_useSearchBox() };
  }
}

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 1, 2021

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.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 1, 2021

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.

@Haroenv
Copy link
Contributor

Haroenv commented Sep 1, 2021

I thought you could use the composition api in vue 2 with @vue/composition-api or something like that?

@eunjae-lee
Copy link
Contributor Author

I thought you could use the composition api in vue 2 with @vue/composition-api or something like that?

You're right. I haven't figured out how to accept that optional dependency for vue 2 yet. So I'm just starting with Vue 3 for now.

const excludeCompositionsAPI = {
load(id) {
if (id.endsWith('/src/compositions/index.js')) {
return '';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Import with '/src/compositions/index.js' gets replaced with an empty string. It's to exclude this line from Vue 2 bundle:

export * from './compositions';

Copy link
Member

Choose a reason for hiding this comment

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

Relying on the build script to modify exports is a bit shady and hard to track.

Can we rather have different entry files? (similar to what React is doing)

rollup.config.js Show resolved Hide resolved
rollup.config.js Show resolved Hide resolved
@eunjae-lee eunjae-lee marked this pull request as ready for review September 7, 2021 12:32
@Haroenv Haroenv requested review from Haroenv, a team and tkrugg and removed request for a team September 8, 2021 07:33
src/util/noop.js Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
@Haroenv Haroenv force-pushed the feat/composition-api branch from 58618c5 to b33cb3b Compare October 1, 2021 12:44
Haroenv
Haroenv previously approved these changes Oct 20, 2021
Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

as I rewrote a bunch of the code, it's best to have someone else review too

Copy link
Member

@francoischalifour francoischalifour left a comment

Choose a reason for hiding this comment

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

It would make sense to start introducing the connector Hooks in stacked PRs and to test them a little.

const excludeCompositionsAPI = {
load(id) {
if (id.endsWith('/src/compositions/index.js')) {
return '';
Copy link
Member

Choose a reason for hiding this comment

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

Relying on the build script to modify exports is a bit shady and hard to track.

Can we rather have different entry files? (similar to what React is doing)

src/compositions/index.js Show resolved Hide resolved
widget = addWidget(
connector,
parent,
isRef(widgetParams) ? widgetParams.value : widgetParams,
Copy link
Member

Choose a reason for hiding this comment

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

Why do we have different data types for widgetParams?

Copy link
Contributor

Choose a reason for hiding this comment

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

in Vue 3, you can only listen to changes from refs, not from plain objects, so we accept both a ref and not a ref, depending on if it needs an update.

This part I'm not 100% sure about, maybe it could accept raw props and put in a ref inside useConnector. Would need to test

Copy link
Member

Choose a reason for hiding this comment

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

Yes I think we should standardize it. (And internally right now we can have an intermediary variable in useConnector to not have to think about all data types whenever we use the props.)

src/compositions/useConnector.js Outdated Show resolved Hide resolved
src/compositions/__tests__/useConnector.js Outdated Show resolved Hide resolved
@Haroenv Haroenv dismissed their stale review October 21, 2021 14:27

issues pointed out

@Haroenv Haroenv mentioned this pull request Nov 4, 2021
@Haroenv Haroenv marked this pull request as draft December 15, 2021 09:18
@Haroenv Haroenv removed the request for review from tkrugg December 15, 2021 09:18
@Baroshem
Copy link

Baroshem commented Jan 5, 2022

Bumping this topic as it looks great!

I just released an Algolia module for Nuxt 3 -> https://algolia-nc.netlify.app/
And I was looking to include vue-instantsearch there as well.

If you can finish this PR and make these components Vue 3 compatible, I can try to import them to the module as well so that the users can use your components in Nuxt 3 as well 😉

@Haroenv
Copy link
Contributor

Haroenv commented Jan 5, 2022

@Baroshem, the components are already vue 3 compatible if you import from vue-instantsearch/vue3, but that's indeed not really properly documented yet

@Baroshem
Copy link

Baroshem commented Jan 5, 2022

@Haroenv I see. I will try then to use them in the module so that they can be imported into the Nuxt 3 right now. Thanks for claryfing!

@Baroshem
Copy link

Hi @Haroenv

I tried yesterday to import vue-instantsearch/vue3 but failed after many attempts to fix the errors. I had issues with

  • "type": "module" in package.json (it was not there so I had to add manually)
  • directory imports
  • external packages imports (instantsearch.js).

I have very little experience with the source code of vue-instantsearch so I am not sure if the changes I was doing are even correct.

Could you please try to install vue-instantseach/vue3 on fresh Nuxt 3 project and see how it works for you?

@Haroenv Haroenv closed this Dec 21, 2022
@Haroenv Haroenv deleted the feat/composition-api branch December 21, 2022 09:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants