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

Make captureRenderTree API public #815

Open
chancancode opened this issue Apr 20, 2022 · 4 comments
Open

Make captureRenderTree API public #815

chancancode opened this issue Apr 20, 2022 · 4 comments

Comments

@chancancode
Copy link
Member

A while back, we added the captureRenderTree API as a more stable (but still private/intimate) API for the Ember Inspector to use. It's fairly well thought out though it didn't really go through the RFC process and did not really consider any use cases outside of the inspector. As of now, the status of the API is that it's officially private/intimate, though since the Ember Inspector uses it it's not going to just go away completely without replacement. It's not a huge priority, but if someone would like to use this API, then let's discuss that here and evolve it towards its final form through the RFC process.

@SergeAstapov
Copy link
Contributor

out of curiosity, is this related to #778

@chancancode
Copy link
Member Author

chancancode commented Apr 20, 2022

The main reason I opened the issue is not related to that PR – I was trying to revive emberjs/ember.js#19069 by @bekzod, however I realized a lot of what made it hard/tricky at the time is to still keep some of the existing private "view utils" functions (getChildViews, etc) working. These functions were originally intended for Ember Inspector, but it doesn't use them anymore since we switched to the newer captureRenderTree() API.

Those functions are not really useful anymore since they only work for classic Ember.Component subclasses. So I set out to deprecate them first. Since there is not a lot of urgency (which is why it took so long to get my attention – sorry! there just always were something more pressing/immediate), I figured it would be easier to remove these APIs (after the next LTS) together with the view registry (which is what the original PR tried to do), so we could delete everything in one go.

However, usually when we deprecate something we also need to offer an alternative. In this case, captureRenderTree() would be the direct replacement. However, captureRenderTree() is also private, and generally we don't want to direct people to using private APIs. In this case, since it is going from a private API to another private API, and it's unclear that anyone actually uses these (and did not already independently decide to switch to captureRenderTree() on their own), it seems okay. But I figured I should make it clear that if anyone is actually using them are aware of this and we could work together on stabilizing this API.

Now, as for the #778, it's at least tangentially related, in that if captureRenderTree() is a private API, it does not logically make a lot of sense to RFC changes to it – either it is a private API and we are free to make the changes as needed, or that the RFC would also have to propose to stabilize it and make it public. Since the #778 seems to be motivated by Ember Inspector use cases, I think the latter is probably fine, but nevertheless this is a perfectly good venue to discuss those changes even if we don't end up needing to formally merge it.

As for the proposal in #778 itself, initially I had the same questions as @rwjblue which he asked on the thread, and I think I am still not super clear on it after re-reading it. This is already getting super long, so I'll try the rest another time and reply on the other PR directly.

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

@chancancode do you think this is worth an RFC? There doesn’t seem to be much uptake here.

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented Jul 23, 2022

I think people get scare of low-level RFCs, sometimes 🙃
having better error / deprecation reporting would be a huge win tho

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

No branches or pull requests

4 participants