Expose vue app instance for extensions

[cnotv]

Summary

Fixes #12628

Occurred changes and/or fixed issues

Logic already exists and exposing the Vue instance should not cause any harm.

Technical notes summary

Areas or cases that should be tested

Areas which could experience regressions

Screenshot/Video

Checklist

• The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
• The PR has a Milestone
• The PR template has been filled out
• The PR has been self reviewed
• The PR has a reviewer assigned
• The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
• The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

[richard-cox]

Could you expand on the use cases this solves?

There's a lot of power from extensions having access to this which is i think why it was omitted. If that was the case could the same use cases be covered by beefing up the plugin api?

[cnotv]

No, in the original configuration, it was specifically passed the attribute and we have cut it out inadvertently. It was not used anywhere and that was the reason we did not notice it.

As I have described in the issue, it would allow to use the Vue App API:

export declare interface App<HostElement = any> {
    version: string;
    config: AppConfig;
    use<Options extends unknown[]>(plugin: Plugin_2<Options>, ...options: Options): this;
    use<Options>(plugin: Plugin_2<Options>, options: Options): this;
    mixin(mixin: ComponentOptions): this;
    component(name: string): Component | undefined;
    component(name: string, component: Component): this;
    directive(name: string): Directive | undefined;
    directive(name: string, directive: Directive): this;
    mount(rootContainer: HostElement | string, isHydrate?: boolean, isSVG?: boolean): ComponentPublicInstance;
    unmount(): void;
    provide<T>(key: InjectionKey<T> | string, value: T): this;
}

I had a case where I needed to extend the app. In the specific case, I needed config.errorHandler to be mapped because this is how it's handled.

[cnotv]

If you are interested in a case where third parties may want to use the extensions, this is one: https://docs.sentry.io/platforms/javascript/guides/vue/#vue-3

[cnotv]

Out of curiosity, could you describe a case where you would not allow this? I could not think of anything specific.

[richard-cox]

No, in the original configuration, it was specifically passed the attribute and we have cut it out inadvertently.

I couldn't find where this was removed, would you be able to provide the commit?

I had a case where I needed to extend the app. In the specific case, I needed config.errorHandler to be mapped because this is how it's handled.
Out of curiosity, could you describe a case where you would not allow this?

I think the plan is to keep a very clean contract between the dashboard via the extension by a well defined API. Anything that the extension needs to change, supplement, override, etc should ideally be via that API. By providing them access to the vueApp (and others objects in internal) we're exposing the dashboard to extensions doing weird and wonderful things. So lots of utility, but also lot of problem. The comment above this block of code roughly covers this concept Plugins should not use these - but we will pass them in for now as a 2nd argument in case there are use cases not covered that require direct access - we may remove access later

[cnotv]

I couldn't find where this was removed, would you be able to provide the commit?

The code was already missing the implementation related to it, so we may talk about 4 years ago?

we're exposing the dashboard to extensions doing weird and wonderful things. So lots of utility, but also lot of problem

For instance?

[richard-cox]

we have cut it out inadvertently

The code was already missing the implementation related to it

I'm not following. It was never wired in, or it was and then removed? Ultimately was looking for breadcrumbs on design decisions.

we're exposing the dashboard to extensions doing weird and wonderful things. So lots of utility, but also lot of problem

For instance

Extensions that depend on root objects could lead to ...

• inadvertently or intentionally breaking the UI in a serious way, error handling primarily but anything else that the dashboard depends or via those objects
• extensions suffer breaking changes given intentional dashboard changes. for instance
  • we update vue and vueApp changes....
  • we replace the vuex store / upgrade it in a breaking way
• extensions suffer breaking changes given unintentional dashboard changes given updates to those objects.
  • extension developers would need to stay up to date with all breaking changes in whatever we pass in, rather than the UI handles those our side of the api unbeknown to extension developer

Restating that architectural point, extensions should interact with the dashboard via well defined api rather than root environment objects, and that the internal object shouldn't be used by extensions and will eventually be removed.

For the record i'm not saying there's no way to integrate sentry via extension, just that it could be done via dedicated api method (maybe a way to inject something into the pipeline in shell/initialize/entry-helpers.js)

If you wish to discuss this further lets have a call with myself, Neil, Alex and Jordan when I'm back.

[cnotv]

All your points are correct, but breaking the application without exposing the instance is still possible.
I already thought about passing around the initialization, but it must still map the error handler, which needs the instance.

This was needed for the hack week, but given the circumstances, I leave it to you guys to plan and find a way that better fits your concept. At least I've raised awareness about it.

Just for the record, the implementation works:

[cnotv]

Closing due to blocking issues.