Skip to content

Conversation

@alexandrtovmach
Copy link
Contributor

Hi there 👋
Played a bit with your package and faced with the same problem as described here #15. Decided to help and here we are 🎉

What I noted is the current structure looks not really well 😕. One react project nested to another it's not a good idea, so probably good to think about a different approach for testing. And one more thing... I found installed testing library, but it's not used as well, and it also affects to described issue. More details here: testing-library/react-testing-library#570

This PR broke the current test environment 🙃 because of the new version of react and hooks. There is some workaround how it's possible to test locally. Let me know if it's interesting for you, but I'd prefer to focus on replacing this structure and testing env, with something more or less modern, or at least with something that will work 😄

Changes:

  • migrated to react hooks
  • updated types to allow pass custom interfaces
  • updated README with typescript example
  • refactored README headings levels

Related

- migrated to react hooks
- updated types to allow pass custom interfaces
- updated README with typescript example
- refactored README headings levels
Fixed issue with setting and initialization attributes like "id", "src".
@alexandrtovmach
Copy link
Contributor Author

alexandrtovmach commented Nov 17, 2020

⚠️⚠️⚠️ DON'T MERGE ⚠️⚠️⚠️

I'm working on required updates

@alexandrtovmach
Copy link
Contributor Author

Funny thing, but I refactored to this, and it works well:

import React, { forwardRef } from 'react';

const reactifyWebComponent = <WebComponentProps,>(WebComponentName: string) => {
  type ReactWebComponentProps = React.HTMLProps<HTMLElement> &
    WebComponentProps;

  return forwardRef<HTMLElement, ReactWebComponentProps>((props, parentRef) => (
    <WebComponentName ref={parentRef} {...(props as ReactWebComponentProps)} />
  ));
};

export default reactifyWebComponent;

@BBKolton Could you take a look and test it before I push it here? Because it seems weird for me, and I fell like I missed something.

@brion-fuller
Copy link
Collaborator

Sorry for the delay. I will take a look into this a bit more. Thank you for helping :)

@tobiyas09
Copy link

Any update on this PR?

@alexandrtovmach
Copy link
Contributor Author

if some maintainer would review it, I'm ready to make this up to date

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

Successfully merging this pull request may close these issues.

3 participants