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

Look into making components compatible with other frameworks #36

Open
andersevenrud opened this issue Oct 18, 2020 · 9 comments
Open

Look into making components compatible with other frameworks #36

andersevenrud opened this issue Oct 18, 2020 · 9 comments

Comments

@andersevenrud
Copy link
Member

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
Copy link

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
Copy link

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
Copy link
Member Author

I see! I forgot about the props & children differences 🤦‍♂️

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 ? 🤔

@ThisIsOstad
Copy link

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
Copy link
Member Author

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

@ThisIsOstad
Copy link

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
Copy link
Member Author

Yeah, you don't need those :)

@ThisIsOstad
Copy link

ThisIsOstad commented Oct 30, 2020

@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
Copy link
Member Author

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

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

No branches or pull requests

2 participants