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

A brief investigation of using SolidJS #6493

Closed
kidonng opened this issue Apr 10, 2023 · 10 comments
Closed

A brief investigation of using SolidJS #6493

kidonng opened this issue Apr 10, 2023 · 10 comments
Labels
meta Related to Refined GitHub itself under discussion

Comments

@kidonng
Copy link
Member

kidonng commented Apr 10, 2023

The use of UI frameworks has been discussed in #4058 and #5779 (comment). This has been on my mind for quite some time and just now I did some brief investigation.

Start from https://www.solidjs.com/guides/getting-started#buildless-options, get these modules first:

npm i solid-js
npm i hyper-dom-expressions -D # For types

Then create a helper function:

import {render} from 'solid-js/web';
import type {JSX} from 'solid-js';

export default function solidFragment(code: () => JSX.Element): DocumentFragment {
	const fragment = document.createDocumentFragment();
	render(code, fragment);
	return fragment;
}

Now we can author our features in Solid:

/* @jsx h */
/* @jsxImportSource solid-js */
/* eslint "react/no-unknown-property": ['error', {ignore: ['class']}] */

// @ts-expect-error Required
import h from 'solid-js/h';
import {createSignal} from 'solid-js'
import {solidFragment} from './solid-fragment'
import select from 'select-dom'

function init() {
	const [count, setCount] = createSignal(0)
	select('.some-class').append(
		solidFragment(() => (
			<button onClick={() => setCount(count() + 1)}>{() => count()}</button>
		)
	))
}

Pros:

  • We can use Solid on a per-file/feature basis, no need to change any build config
  • Still use JSX and types are kinda fine
  • Performance is non-issue
  • Interoperable since Solid renders to native elements
  • Ergonomic is the final goal

Cons:

  • More ceremony just to attach elements
  • Buildless trade-offs as mentioned in Solid docs
  • Haven't figured out how to use @primer/octicons-react yet
  • Slightly increased bundle size (~7 KB)

I'm sharing my thoughts here in case we do find a UI framework helpful.

@kidonng kidonng added meta Related to Refined GitHub itself under discussion labels Apr 10, 2023
@fregante
Copy link
Member

This has been on my mind for quite some time

Same here, just yesterday I was annoyed at home a feature would benefit from this (I don't remember which)

  • Still use JSX and types are kinda fine

Make sure that "kinda fine" turns into "CI is green" first 😃

  • Haven't figured out how to use @primer/octicons-react yet

You mean "in Solid JSX features"? It could be patched via a dom-chef-based proxy file… maybe?

import React from 'dom-chef';
import {XIcon} from '@primer/octicons-react';

export () => <XIcon/>
  • Ergonomic is the final goal

Indeed, it would be good to find a feature that would benefit from this kind of interactivity and actually port it to Solid. Because in my last attempt I also got "90% there" and then had to abandon because that 10% was too much.

@kidonng
Copy link
Member Author

kidonng commented Apr 10, 2023

Make sure that "kinda fine" turns into "CI is green" first 😃

I said kinda because in my experiment (and example) the types are falling back to @types/react. I didn't see any type error but that was a very simple try.

I did meet some errors with /* @jsxImportSource solid-js */ though, maybe it was due to something else and is resolvable.

You mean "in Solid JSX features"? It could be patched via a dom-chef-based proxy file… maybe?

That's what I thought too, but it could become tedious. Or maybe this work:

import {SomeIcon} from '@primer/octicons-react'

const element = <div>{SomeIcon()}</div>

Or something like that since it is a function and we have aliased react.

@fregante
Copy link
Member

fregante commented Apr 10, 2023

SomeIcon()

Yes I think that's the right way. React is already aliased globally to dom-chef so it should't matter where you import @primer/octicons-react. At most you'll need

import domChef from 'dom-chef'
import {SomeIcon} from '@primer/octicons-react'

const element = domChef.createElement(SomeIcon)

but I don’t think this is necessary. I just forget how these functions are handled here.

@fregante
Copy link
Member

So yeah, this would be welcome after:

  • one feature is converted to use it
  • icons can be used in it
  • CI passes without ifs and buts

@kidonng
Copy link
Member Author

kidonng commented Apr 11, 2023

This is enough to make icons work:

<SomeIcon {...SomeIcon.defaultProps}/>

@fregante
Copy link
Member

You know what has been killing me for years? That staleWhileRevalidate does not update the DOM after a new version has been fetched. If we have a way to have dynamic elements, I could create a (signal-based?) API that updates when the new value is fetched.

A hidden example for this would be the CSS hotfixes:

  1. Inject cached CSS immediately to avoid FUOC
  2. Update it when the update comes in (to avoid having to reload the page)

Related:

@fregante
Copy link
Member

sync-pr-commit-title would be another feature that has "complex" dependencies and may benefit from better state management, but this requires further research so it's best not to start with this.

  • the PR commit title field depends on the PR title
  • the "will update" note depends on:
    • the PR title
    • the PR commit title
    • whether the user pressed "cancel"

There's no straightforward way to do this, we don't even depend on the PR title changing at the moment.

@fregante
Copy link
Member

fregante commented Apr 26, 2023

If you manage to implement a minimal version, I think this could be the missing piece to

Instead of removing and recreating that button, we'd just have a dynamic component whose href and disable attributes are changed depending on the URL.

Following the demo on their site, I think the correct setup for this would look like:

import { render } from "solid-js/web";
import { onCleanup, createSignal } from "solid-js";

const CountingComponent = () => {
  const [count, setCount] = createSignal(0);
  const interval = setInterval(() => {
    setCount((count) => count + 1);
  }, 1000);
  onCleanup(() => clearInterval(interval));
  return <div>Count value is {count()}</div>;
};

function add(anchor) {
  render(() => <CountingComponent />, solidFragment(anchor));
}

function init() {
  observe(".some-class", add);
}

If you can make the above work, we can figure out the icons later.

And then for download-folder-button the component would look like:

const DownloadFolderButton = () => {
  const [url, setUrl] = createSignal(false);
  const interval = setInterval(
    // Or a URL watcher
    () => {
      setUrl(isRepoTree() && !isRepoRoot() && createDownloadUrl(location.href));
    },
    100
  );
  onCleanup(() => clearInterval(interval));
  return (
    <a hidden={!url()} href={url()}>
      Download
    </a>
  );
};

@fregante
Copy link
Member

fregante commented May 25, 2023

This might be a better solution, depending on how it deals with raw DOM elements (e.g. dom-chef generated icons)

https://github.com/vanjs-org/van

Judging by the comments on HN, there’s no shortage of options:

https://news.ycombinator.com/item?id=36067983

We just have to pick one that deals well with:

  • append/remove at any point on the page
  • use of existing DOM (JSX icons or whatever DOM node we pass it as child)

@fregante
Copy link
Member

fregante commented Jun 1, 2023

Closing for now. Feel free to send a minimal PR, but I think SolidJS might not be the right choice here. Anything using JSX is probably going to be problematic due to the icons.

I think the ideal solution would be something based on Web Components because they're built specifically to be self-contained:

  • they can be injected as native elements and even via JSX, e.g. .append(<my-active-component/>)
  • have native lifecycle events that deal with "activate on insertion" and "deactivate on page change"
  • they can be altered via attributes (which is probably less of an issue if the component is truly self-contained)
  • they accept any DOM element via slots:
    <my-active-component><XIcon/> Click me</my-active-component>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Related to Refined GitHub itself under discussion
Development

No branches or pull requests

2 participants