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

Don't merge: upgrade simple-dom #760

Closed
wants to merge 1 commit into from
Closed

Conversation

chancancode
Copy link
Contributor

@chancancode chancancode commented Dec 30, 2017

The actual implementation of simple-dom (now fully typed) is not compatible with Simple.*. Specifically, it does not support namespaces. Removing them from Simple.* yields around type 20 errors:

render-test_ts_ _glimmer

Most of these are in tests. However, I do think there is a real issue. Specifically, in SSR mode, we are wrongly assuming we can call NS methods like createElementNS. I don't know if this causes problems in practice – maybe a combination of things causes these things to never be called in practice, but we should get the types to more accurately reflect reality.

Also, since simple-dom is now fully typed, and is a strict subset of DOM, do we still need Simple.*?

@tomdale
Copy link
Contributor

tomdale commented Nov 28, 2018

I think this is superseded by #866.

@tomdale tomdale closed this Nov 28, 2018
@tomdale tomdale deleted the upgrade-simple-dom branch November 28, 2018 22:14
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.

2 participants