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

Bring back OptionalRootable trait #6833

Closed
frewsxcv opened this issue Jul 29, 2015 · 4 comments
Closed

Bring back OptionalRootable trait #6833

frewsxcv opened this issue Jul 29, 2015 · 4 comments

Comments

@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Jul 29, 2015

From this thread on Reviewable

@Ms2ger: .map(|s| s.root()) should be just .root()
@frewsxcv: OptionalRootable (which presumably made this method on Option possible) was removed in #6150
@Ms2ger: Bah. File an issue to bring it back.

@frewsxcv
Copy link
Member Author

@frewsxcv frewsxcv commented Jul 29, 2015

/cc @michaelwu since they might have more context as to why the trait was removed in the SM-up

@michaelwu
Copy link
Contributor

@michaelwu michaelwu commented Jul 30, 2015

Needing to call root() at all is a bit unsafe. What you really want to do here is call get_rooted() on next_sibling/first_child. See #6684 . OptionalRootable just papers over the brokenness of dealing with raw JS<T>.

@frewsxcv
Copy link
Member Author

@frewsxcv frewsxcv commented Apr 5, 2016

Is this still desired?

cc @Ms2ger

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Apr 5, 2016

Meh, I guess.

@Ms2ger Ms2ger closed this Apr 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.