feat(horizontal-slider): change renderer implementation to VDOM #28

shortcuts · 2021-06-03T12:54:28Z

Follow up of the previous PR: algolia/ui-components#27

This is a rework of the rendering system of the Horizontal Slider UI components to an agnostic virtual DOM implementation.

Problems

Here's a list of the problems we have with the current architecture:

Bundle size. The JavaScript package embeds the complete preact/compat package which exports the whole React API. (We now only rely on preact and preact/hooks.)
Not future-proof. The current React/Preact aliasing is not going to allow us to bring support for Vue, React Native, and all VDOM-based framework.
Bridge between JavaScript packages and UI components. The bridge between UI components like Horizontal Slider and the Recommendations component is challenging because we're not in control of the view prop rendering framework.
Tooling. The tooling is difficult to get working because we have to alias the dependencies in all our toolchain (Parcel, Jest, Rollup, Babel).

Architecture

Current

The previous packages main entry point was react-horizontal-slider, where the logic and the rendering happened. The js-horizontal-slider package relied on react-horizontal-slider with an alias from react to preact/compat.

New

We now have these packages:

packages
├── horizontal-slider-js
├── horizontal-slider-react
├── horizontal-slider-theme
└── horizontal-slider-vdom

`horizontal-slider-js`

horizontal-slider-js is an implementation of horizontal-slider-vdom with Preact (similar front-end framework that we use in Autocomplete, InstantSearch and the recommendations-js package). It specifies horizontal-slider-vdom with the Preact renderer.

API

horizontalSlider

`horizontal-slider-react`

horizontal-slider-react is an implementation of horizontal-slider-vdom with React.

API

HorizontalSlider

`horizontal-slider-theme`

Package that hosts the HorizontalSlider component default theme.

`horizontal-slider-vdom`

horizontal-slider-vdom is an agnostic package that allows to build components given a renderer. A renderer is composed of { createElement, Fragment }. It's purely the UI part and creates uncontrolled components, lacking any state and lifecycle functionalities.

API

createHorizontalSliderComponent
generateHorizontalSliderId: the function to initialize the sliderIdRef ref with.
updateNavigationButtonsProps: the function to wrap in an effect, which will update the current value of the refs.

Changes

`container`

The container props for the recommendations-js and horizontal-slider-js packages are now optional. When undefined, we will directly return the component instead of rendering it in its container.

This allows us to:

Leverage the view prop of the recommendations-js components with the HorizontaSlider.
Render a fallbackComponent from the recommendations-js library in the same container as its parent.

Usage

frequentlyBoughtTogether({
  container: '#frequentlyBoughtTogether',
  searchClient,
  indexName,
  // ...
  fallbackComponent() {
    return relatedProducts({
      searchClient,
      indexName,
      view: horizontalSlider,
      // ...
    });
  },
});

Other packages in the future

When the time comes, we'll be able to create horizontal-slider-vue with the same strategy as horizontal-slider-js and horizontal-slider-react. We could also expose this component for React Native in the future.

Next steps

Deprecate the previous packages
Update rollup bundle config

francoischalifour

Looks good—love the optional container pattern!

I've got a concern about the ref compatibility with a future Vue package though.

examples/js-demo/index.html

packages/horizontal-slider-js/README.md

packages/horizontal-slider-vdom/src/index.ts

francoischalifour · 2021-06-03T13:25:54Z

packages/horizontal-slider-vdom/src/types/FrameworkProps.ts

+type Ref<TElement> = {
+  current: TElement;
+};


I'm wondering how that will play out with Vue because their refs are strings.

Since we will be in control of the Vue implementation, we could define a fallback type for it

francoischalifour · 2021-06-03T13:27:12Z

packages/horizontal-slider-vdom/src/types/Renderer.ts

@@ -0,0 +1,36 @@
+export type Pragma = (


Next component that we create we might want have a shared/common package for these types.

Yep, I can already see multiple use cases for a shared package!

packages/recommendations-js/README.md

README.md

package.json

Haroenv · 2021-06-03T14:30:01Z

packages/horizontal-slider-js/README.md

@@ -1,13 +1,13 @@
-# `@algolia/ui-components-js-horizontal-slider`
+# `@algolia/ui-components-horizontal-slider-js`


much better name, alternative would have been js-ui-components- horizontal-slider, but this format works well

Haroenv · 2021-06-03T14:31:17Z

packages/horizontal-slider-js/src/horizontalSlider.tsx

+      nextButtonRef,
+      previousButtonRef,
+    });
+  });


should this happen on every change?

Since this call will set update the hidden boolean to hide/show our arrows, yes. Did you meant to add the concerned refs to the dependencies?

Haroenv · 2021-06-03T14:32:21Z

packages/horizontal-slider-js/src/horizontalSlider.tsx

+  environment = window,
+  ...props
+}: HorizontalSliderProps<TObject> & EnvironmentProps) {
+  const children = <HorizontalSlider {...props} />;


should this be called children or component (it's only one element, maybe child)?

We choosed children to keep it consistent with the return of our recommendations components

component also makes sense to me. Both are fine IMO.

Haroenv · 2021-06-03T14:33:23Z

packages/horizontal-slider-react/src/HorizontalSlider.tsx

+      listRef={listRef}
+      nextButtonRef={nextButtonRef}
+      previousButtonRef={previousButtonRef}
+      sliderIdRef={sliderIdRef}


I wonder if this should be a single refs object?

probably not due to the diff annoyances that causes

I feel like this is easier to understand as single refs, what would your implementation looks like?

refs={{ nextButton: nextButtonRef, previousButton: previousButtonRef, sliderId: sliderIdRef }} or something similar

Haroenv · 2021-06-03T14:34:57Z

packages/horizontal-slider-vdom/src/HorizontalSlider.tsx

+let lastHorizontalSliderId = 0;
+
+export function generateHorizontalSliderId() {
+  return `uic-horizontal-slider-${lastHorizontalSliderId++}`;


does this generate stable ids for use in server side rendering?

This is the same logic as we used in autocomplete but I didn't tried it yet with SSR!

For this to be SSR-safe, we need to accept an id prop in all our components. You would need to generate the id on the server and pass it to the client. (Autocomplete allows this.)

We've planed to add this later. But as it is now, it's not SSR-safe.

(Other interesting resources could be the new React's useOpaqueIdentifier.)

Haroenv · 2021-06-03T14:38:33Z

packages/recommendations-js/README.md

+      <pre>
+        <code>{JSON.stringify(item)}</code>
+      </pre>


can you use it without preact (document.createElement) too, or does that not make sense?

That would be either VDOM or HTML strings (see algolia/ui-components#27 (comment)).

Co-authored-by: Haroen Viaene <hello@haroen.me>

Haroenv

all good for me

francoischalifour and others added 25 commits June 1, 2021 18:14

feat: change renderer implementation to VDOM

ace52ec

chore: update build system

312f679

chore: update bundle sizes

a7af784

docs: update readmes

dc7c799

feat: update user agents

4133cc2

refactor(js): extract useAlgoliaAgent

09fc10e

feat: update reco return format

bb2e921

chore(ci): update Circle config

666c5fb

chore(ci): add packages to CI

d187e8c

test(core): add tests for getRecommendations

de7ed07

feat: update translations

00b1952

docs: update demo deps

431dbce

fix: update translation title

a2ac8d5

feat(horizontal-slider): change renderer implementation to VDOM

49a8a3e

Merge branch 'feat/vdom' into feat/vdom-pt2

ac461d7

docs(readme): mention core and vdom packages

10272d4

chore(build): add @babel/plugin-transform-runtime

e0f94b0

refactor: rename GetRecommendationsResult

38f20bc

fix: update README

0b06102

refactor: rename DOMRef type

ec80f25

refactor: use types from core

2fba5ac

feat(vdom): export types

128305e

fix: remove unneeded dependency and update package.json

cef3bcc

doc: make container optional

0d0fd57

fix: typos

4b65820

shortcuts requested review from francoischalifour and Haroenv June 3, 2021 12:54

Merge branch 'feat/vdom' into feat/vdom-pt2

4a81900

francoischalifour approved these changes Jun 3, 2021

View reviewed changes

fix: remove unused container

3e49a8b

shortcuts added 2 commits June 3, 2021 15:48

doc: remove unneeded pipe

0aef918

fix: sort files ascending

e685233

Haroenv reviewed Jun 3, 2021

View reviewed changes

Update README.md

43ec454

Co-authored-by: Haroen Viaene <hello@haroen.me>

francoischalifour approved these changes Jun 8, 2021

View reviewed changes

Base automatically changed from feat/vdom to next June 8, 2021 09:14

Merge branch 'next' into feat/vdom-pt2

d39128f

Haroenv approved these changes Jun 8, 2021

View reviewed changes

remove preact alias

081b051

shortcuts merged commit e297a92 into next Jun 8, 2021

shortcuts deleted the feat/vdom-pt2 branch June 8, 2021 09:35

shortcuts mentioned this pull request Jun 8, 2021

feat(component): pass createElement and Fragment components #26

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(horizontal-slider): change renderer implementation to VDOM #28

feat(horizontal-slider): change renderer implementation to VDOM #28

shortcuts commented Jun 3, 2021

francoischalifour left a comment

francoischalifour Jun 3, 2021

shortcuts Jun 3, 2021

francoischalifour Jun 3, 2021

shortcuts Jun 3, 2021

Haroenv Jun 3, 2021

Haroenv Jun 3, 2021

shortcuts Jun 8, 2021

Haroenv Jun 3, 2021

shortcuts Jun 3, 2021

francoischalifour Jun 3, 2021

Haroenv Jun 3, 2021

Haroenv Jun 3, 2021

shortcuts Jun 3, 2021

Haroenv Jun 3, 2021

Haroenv Jun 3, 2021

shortcuts Jun 3, 2021 •

edited

Loading

francoischalifour Jun 3, 2021

Haroenv Jun 3, 2021

francoischalifour Jun 8, 2021

Haroenv left a comment

		@@ -1,13 +1,13 @@
		# `@algolia/ui-components-js-horizontal-slider`
		# `@algolia/ui-components-horizontal-slider-js`

feat(horizontal-slider): change renderer implementation to VDOM #28

feat(horizontal-slider): change renderer implementation to VDOM #28

Conversation

shortcuts commented Jun 3, 2021

Problems

Architecture

Current

New

horizontal-slider-js

horizontal-slider-react

horizontal-slider-theme

horizontal-slider-vdom

Changes

container

Other packages in the future

Next steps

francoischalifour left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shortcuts Jun 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Haroenv left a comment

Choose a reason for hiding this comment

`horizontal-slider-js`

`horizontal-slider-react`

`horizontal-slider-theme`

`horizontal-slider-vdom`

`container`

shortcuts Jun 3, 2021 •

edited

Loading