salesforce / lwc

⚡️ LWC - A Blazing Fast, Enterprise-Grade Web Components Foundation
https://lwc.dev
Other
1.63k stars 394 forks source link

Remove `Elm.$$ShadowRoot$$` from synthetic shadow #1061

Closed caridy closed 5 years ago

caridy commented 5 years ago

This feature is not longer needed, and instead any test lib can rely on Elm.shadowRoot, which is supposed to be always "open" in test-mode.

pmdartus commented 5 years ago

@trevor-bliss What do you think about making the getShadowRoot method from the test-utils return elm.shadowRoot and deprecate this method from the package. We can still keep the package around in case we add new utils in the future.

We should also start publicizing internally that we deprecate this method and that they can now migrate to element.shadowRoot. We should also add a warning message in the console. I found more than 1500 occurrences of this API in core, so we should start soon. 🤣

diervo commented 5 years ago

@trevor-bliss I think this could be a prefect opportunity to try our "deprecation strategy". @Gr8Gatsby @bkusumo would you agree?

Let' see if Scott hold the fort if the right announcement is made.

trevor-bliss commented 5 years ago

@pmdartus @diervo Sounds great, I would love to deprecate this API. This shouldn't have been used or documented anywhere outside of core so it should be an "internal only" deprecation.

@bkusumo @Gr8Gatsby Do we have specific timelines laid for this? Happy to make the code change and initial chatter post this week.

Gr8Gatsby commented 5 years ago

Are there more APIs beyond this in the same category? We can class this in the preparing for open source work. But we need to make sure we do a review of all APIs. I have a user story for the team to review APIs before going OSS

Get Outlook for iOShttps://aka.ms/o0ukef


From: Trevor notifications@github.com Sent: Wednesday, February 20, 2019 09:59 To: salesforce/lwc Cc: Kevin Hill; Mention Subject: Re: [salesforce/lwc] Remove Elm.$$ShadowRoot$$ from synthetic shadow (#1061)

@pmdartushttps://github.com/pmdartus @diervohttps://github.com/diervo Sounds great, I would love to deprecate this API. This shouldn't have been used or documented anywhere outside of core so it should be an "internal only" deprecation.

@bkusumohttps://github.com/bkusumo @Gr8Gatsbyhttps://github.com/Gr8Gatsby Do we have specific timelines laid for this? Happy to make the code change and initial chatter post this week.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/salesforce/lwc/issues/1061#issuecomment-465688226, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AGXmaKy_Pm9ZvuCVkRX9wqCnnmN6IDy9ks5vPY0CgaJpZM4bEXZn.

pmdartus commented 5 years ago

No, it's the only API in the @lwc/test-utils package.

pmdartus commented 5 years ago

Fixed with #1069 and #1071