os-js / osjs-gui

OS.js GUI Module
https://manual.os-js.org/v3/
Other
18 stars 10 forks source link

Look into making components compatible with other frameworks #36

Open andersevenrud opened 4 years ago

andersevenrud commented 4 years ago

Since other frameworks like React and Vue compose components in mostly the same way (behind the scenes) it should be possible to convert most components -- or maybe even there's a way to use them directly.

One component that comes to mind which cannot be directly converted is the ListView and IconView components because they use some glue code to bind state and actions in an app. Without this a user needs to write some extra code to bootstrap these components.

ThisIsOstad commented 4 years ago

I have time to work on this issue and add React components. Please let me know if it's OK and you're not working on it.

I looked at this project and currently, I think it should be like this:

  1. Change code style to JSX so that we can use common codes between Hyperapp and React
  2. Change babel config based on an environment variable (say UI_FRAMEWORK === "react" || "hyperapp") to transform JSX to either h or React.createElement.
  3. Change webpack and rollup configs to handle where codes are different based on this environment variable: My proposal is to have x.js file where codes are common and use x.react.js & x.hyperapp.js where codes are different. In this way, we can easily add other frameworks in the future.
  4. I don't know what's the best way to handle imports and maybe we should have something like import {X} from '@osjs/gui/react'.
ThisIsOstad commented 4 years ago

Well, I played a bit with this. There are a few problems in using common components.

The most important one is that the children prop is passed as the second argument in Hyperapp components.

The second problem is that Hyperapp uses lowercase props (like class, onclick, etc.) which can be handled by this babel plugin but still, it's better to support lowercase props in Hyperapp components and camelCase props in React components. For example, the Box component should receive className in React and class in Hyperapp.

We can handle both of these problems in runtime but it has a little overhead in runtime. IMO it's better to separate all files like Box.js (for Hyperapp as the default framework) and Box.react.js for React. It also makes it easier to add other frameworks like Vue, because it doesn't need to maintain common codes anymore.

andersevenrud commented 4 years ago

I see! I forgot about the props & children differences :man_facepalming:

We can handle both of these problems in runtime but it has a little overhead in runtime. IMO it's better to separate all files like Box.js (for Hyperapp as the default framework) and Box.react.js for React. It also makes it easier to add other frameworks like Vue, because it doesn't need to maintain common codes anymore.

Yes, a separation would be best. Maybe even a separate package entirely to reduce the size of the dependency. ESM tree shaking takes care of it on build, but maybe it's better to not make the @osjs/gui package any larger than it has to be ? :thinking:

ThisIsOstad commented 4 years ago

With a separate package, CSS files will be repeated, and every pull request in this package needs another pull request in the other package. Any way I can do it if you initiate the project.

andersevenrud commented 4 years ago

Actually, the CSS from @osjs/gui is added globally in an installation by default. Only components are baked into the apps.

ThisIsOstad commented 4 years ago

Thanks, I've started working on the React version in a fork here. Considering required CSS is added globally, can I totally remove CSS files?

Also, I guess you are using src/contextmenu.js and src/provider.js globally too. Can I remove them too?

andersevenrud commented 4 years ago

Yeah, you don't need those :)

ThisIsOstad commented 4 years ago

@andersevenrud I created a pull request on the fork. It'll be great if you can take a look at it.

I tried not to refactor or change any logic and only converted codes from Hyperapp to React. However, please let me know if you think it's better to change some parts of the code.

andersevenrud commented 4 years ago

I'm a bit busy with work, but I'll look at this and get back to you ASAP :)