Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

Optimize React-in-Vue of immediate react children. #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

phillbaker
Copy link
Contributor

@phillbaker phillbaker commented May 25, 2018

This is a proof of concept for handling a special case of React components as children of React components in Vue. It works by creating a registry of React components and then checking to see if components are parent/children.

Vue's built-in resolution with name checking isn't available as a utility, so included similar code.

  • should add a test or two covering this behavior

Ref: #19.

This is a proof of concept for handling a special case of React components as children of React components in Vue.

[Vue's built-in resolution](https://github.com/vuejs/vue/blob/05299610ea3e89ddbcfe4d8ede0c298223766423/src/core/vdom/create-element.js#L105) with [name checking](https://github.com/vuejs/vue/blob/19552a82a636910f4595937141557305ab5d434e/dist/vue.runtime.common.js#L1491) isn't available as a utility, so included similar code.

Ref: akxcv#19.
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Oops! Something went wrong! :(
ESLint: 4.18.2.
ESLint couldn't find the plugin "eslint-plugin-vue". This can happen for a couple different reasons:
1. If ESLint is installed globally, then make sure eslint-plugin-vue is also installed globally. A globally-installed ESLint cannot find a locally-installed plugin.
2. If ESLint is installed locally, then it's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
    npm i eslint-plugin-vue@latest --save-dev
If you still can't figure out the problem, please stop by https://gitter.im/eslint/eslint to chat with the team.

@phillbaker
Copy link
Contributor Author

@akxcv any thoughts here?

@akxcv
Copy link
Owner

akxcv commented Jun 21, 2018

Hi! I skimmed through your code a few times, and, frankly, it's quite hard to understand what is going on. Could you please lay out the general strategy in plain English?

I think it should be possible to detect whether the component is from Vue or from React in the wrappers themselves (right now they are just blindly wrapping whatever we pass). Correct me if I'm wrong, but it is much simpler than using a registry of some sort.

@phillbaker
Copy link
Contributor Author

@akxcv thanks for taking a look.

I think it should be possible to detect whether the component is from Vue or from React in the wrappers themselves (right now they are just blindly wrapping whatever we pass). Correct me if I'm wrong, but it is much simpler than using a registry of some sort.

I initially thought so and #44 may help here, but I wasn't able to do this in the wrapper objects themselves because all components look the same - it seems only possible to determine the true type of a component in the plugins. E.g. in Vue, all react components are converted to vue components in VuePlugin.

I skimmed through your code a few times, and, frankly, it's quite hard to understand what is going on. Could you please lay out the general strategy in plain English?

  • in VuePlugin, maintain a registry of which components are actually react components (under the hood)
  • in React.js look up the component in the registry by any of the various names it could be referenced
    • if the component is a react component and all of its direct children are react components use react directly to render the children
  • update Vue.js to use unique component names to avoid Vue reusing instances (frankly I don't understand the Vue internals enough to understand why this is necessary)

This was definitely a proof of concept and happy to refactor / take a different direction!

@akxcv
Copy link
Owner

akxcv commented Jun 26, 2018

Ah, the problem is that the Vue plugin is detecting a React component and instantly wrapping it into a React wrapper, right?
Again, sorry for taking a long time...

@phillbaker
Copy link
Contributor Author

phillbaker commented Jun 26, 2018 via email

@akxcv
Copy link
Owner

akxcv commented Jun 26, 2018

Then, I think, we need to rethink the whole flow of Vuera to be more robust, because the current algorithm is kind of hacky. I'll try to think about it soon.

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.

3 participants