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

Give insertBefore a Node or null as anchor #2573

Closed
EmilTholin opened this issue Apr 26, 2019 · 2 comments · Fixed by #2616
Closed

Give insertBefore a Node or null as anchor #2573

EmilTholin opened this issue Apr 26, 2019 · 2 comments · Fixed by #2616

Comments

@EmilTholin
Copy link
Member

EmilTholin commented Apr 26, 2019

referenceNode is not an optional parameter -- you must explicitly pass a Node or null. Failing to provide it or passing invalid values may behave differently in different browser versions.

This throws an error in IE9 and 10, so it might be worth defaulting the anchor component option to null:

mount_component(component, options.target, options.anchor || null);

Or possibly defaulting it at insert:

export function insert(target, node, anchor) {
  target.insertBefore(node, anchor || null);
}

I'm not sure what's best, so I thought I'd open an issue and discuss it before creating a PR.

@AlbertMarashi
Copy link

@Rich-Harris @EmilTholin having some errors with this
image

It's happening during hot-reload changes, but it only seems to be happening with the root element. Any idea why?

@AlbertMarashi
Copy link

AlbertMarashi commented May 3, 2020

I left some notes on https://discordapp.com/channels/457912077277855764/653341885674553345/706344295996588084

It's trying to insert a node at the target (which exists), but the anchor node (which is supposed to be in the target) is not in the target
with target.insertBefore(newNode, anchor || null)
anchor is removed from the target
so it's trying to inset a node before the target's anchor child, but the anchor isn't a child

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 a pull request may close this issue.

2 participants