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.

Sign up for GitHub

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

Jump to bottom

Parcel appendTo appears to be broken #157

Open
alanquigley-toast opened this issue Nov 23, 2022 · 4 comments · May be fixed by #199
Open

Parcel appendTo appears to be broken #157

alanquigley-toast opened this issue Nov 23, 2022 · 4 comments · May be fixed by #199
Labels
bug

Comments

@alanquigley-toast
Copy link
Contributor

alanquigley-toast commented Nov 23, 2022 •
edited

When I set appendTo on a Parcel, I get a mount failure. The error leads back to the following line:

single-spa-react/src/parcel.js

Line 49 in c2cf485

this.parcel = mountParcel(this.props.config, {

When appendTo is set, the following React.createElement call never happens, so this.el is never set.

single-spa-react/src/parcel.js

Line 104 in 13e9fc8

return React.createElement(

When the call to getParcelProps function is made on line L51 the domElement is internally set to undefined, and the spread overrides the created domElement on L50

single-spa-react/src/parcel.js

Line 155 in 13e9fc8

const parcelProps = { ...this.props, domElement: this.el };

My question is, does this look like a bug, or is it related to my use-case currently. It seems like a bug.

Swapping lines L51 with L50 so that the newly generated domElement is added after the getParcelProps call does resolve the issue.

Happy to create a PR if required.
@SeverS
Copy link

SeverS commented Oct 12, 2023 •
edited

Hi, I've encountered this issue as well, any plans on fixing it? I can also open a pr if needed.

@SeverS
Copy link

SeverS commented Oct 12, 2023

@alanquigley-toast @MilanKovacic sorry for pinging you but I need to decide if I proceed with a workaround or wait for a fix

@MilanKovacic
Copy link
Collaborator

Hi, PRs are always welcome. Feel free to submit one. If possible, also set up a minimal reproduction repository.

This was referenced Oct 14, 2023
Closed
Open
@SeverS
Copy link

SeverS commented Oct 14, 2023

Thanks @MilanKovacic, I've opened pr #199 and also added reproduction repository in the description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix parcel appendTo SeverS/single-spa-react
Swap L51 and L50 SeverS/single-spa-react
3 participants
@SeverS @MilanKovacic @alanquigley-toast