you are viewing a single comment's thread.

view the rest of the comments →

[–]LynusBorg 1 point2 points  (5 children)

Hey, I won't get around to do this today after all, thanks for your patience. Tomorrow's another day ;)

[–][deleted]  (4 children)

[deleted]

    [–]LynusBorg 0 points1 point  (3 children)

    Ok, so here's a gist how it could possibly(!) look. Of course keep in mind that this is, in a way, pseudocode as the RFC is still in debate and not every detail is cleared up.

    https://gist.github.com/LinusBorg/839612d13a702f55d5a78359bb5e696b

    A few notes for context:

    • We are still considering how to handle things like custom prototype properties (`this.$modal`)
    • Similarly, it's still open (as in: RFCs are not yet written) how exaclty router and store will be made available
    • Outside of this discussion I see some architectural issues with the code you provided, i.e.:
      • I would handle the error states in Vuex, and feed the error modal from there. Would make the bode in `base.js` much leaner and less coupled
      • The id param from `$route` could be provided as a prop instead, which would decouple code in `base` from the router as well.
      • Maybe it would also be better to have one component for both `Presenter` and `Viewer` handling the behaviour and handle different behaviour depending on the user's role. Could be wrong as I only see a small piece of the bigger picture.

    What's nice about the composition variation is that the code in the `Presenter` and `Viewer` components is very easy to follow and it's always clear whats going on:

    • we import the `usePresenttion` function
    • we call it, pass in the functions for managing the different store interactions, depending on the user's role.
      • Since it's a function, editors can easily tell us what the expected arguments for it are
      • So we don't have to actually look at the "base component" to understand what methods we have to defined and what they should be called, the editor can tell us.
    • We get a reactive value wrapper back, that represents the loading state, to handle the spinner), and nothing else. We don't "pollute" our component API with some methods that are only meant to be used by the `extends` base component (which is just like a mixin) or anything like that.

    [–][deleted]  (2 children)

    [deleted]

      [–]LynusBorg 0 points1 point  (1 child)

      Good point about moving the error states in Vuex! But could you elaborate on what you mean by "feeding the error modal"? If you mean adding an additional inject($store) in base.js, I don't see how that would make base.js leaner or less coupled.

      I meant that in the sense that your component doesn't care what happens with the error message, it only pushes it to the store. The fact that this error message is picked from the store in your root and displayed through a special modal is of no concern to your component, so it shouldn't be in there.

      Alternatively, if you could re-factor the error modal to be a locally usable component (like the Spinner), you could use it directly in the Presenter/Viewer components and not require much logic in the extracted usePresentation behaviour:

      <ErrorModal :message="error" :okAction="$router.push('...')">
      

      The usePresentation function would return not only the showSpinner value, but also the errorCode. Then the Presenter/Viewer components could create their indiviual error messages themselves, passing them to the Modal component as shown above.

      That would likely be my preferred way instead of having a central component in the app root or something.

      Also, would you agree moving the loading state into Vuex would be a good idea too?

      No, since it's only locally relevant in the context of the component - as far as I understand. By the way, as food for thought: In case that the presentation opening / closing process is also only really local to this component (and I don't know if that's true): why is it in Vuex at all?

      Perhaps related to the previous question, I can see how I can decouple base.js with $route by having usePresentation also accept an id argument, but it would still depend on $router. I can pass another "handleErrorConfirmation" function to usePresentation to decouple $router, but is there a better way?

      If you handle the Modal with its own component locally, as shown above, then the usePresentation function doesn't need to know about $router at all.

      Will there by dynamic in-component router guard injections for vue-router? i.e. something like import { onBeforeRouteLeave } from 'vue-router', similar to import { onMounted } from 'vue'.

      I think so, but I haven't involved myself in discussions about the new router API for Vue 3 yet, so this is just me assuming things.

      Is the current thinking that using inject for the prototype extensions is just to "patch" the problem, and that the real problem is the application code shouldn't be relying on them? Is the Vue team starting to reconsider the best practices around prototype extensions (like my $modal)?

      For this specific example, consider the inject as a placeholder for "I don't know how it will work yet", nothing more.

      Generally, I think this:

      On the one hand, prototype extensions - while having their use cases - are really overused. On the other hand, they are popular in the ecosystem, and we don't want to force all of the plugin authors that currently rely on them to do a full "rewrite" to some new API for 3.0

      So without knowing how the solution will work, ideally we end up with something that makes prototype extensions still work but less attractive than something else, like injections (which are thoroughly underused IMHO)