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

[Feature Request] - A new enhanceIslands hook to add plugins and other setup to Islands #277

Open
TechAkayy opened this issue Sep 4, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@TechAkayy
Copy link
Collaborator

TechAkayy commented Sep 4, 2024

Expanding one of the ideas from here - #276. I want to run this past the community before submitting a PR. Please share your thoughts.

Is your feature request related to a problem? Please describe.

The current enhanceApp only applies to the "outer" shell app that is used during development (for a nice HMR experience) and during build time to statically generate the app (as explained by @ElMassimo here - #213 (comment)). This is useful for certain use cases (like the usage of Vue plugins), such as what's discussed in this i18n support thread - #6. However, this "outer" app won't be available in the built app.

Currently, to use Vue plugins inside interactive Islands, the only way is to add the plugin inside the island, as discussed in these threads: #207, #259, #6 (reply in thread), #118 (comment), #213 (comment), #61 (comment). For example:

<script setup lang="ts">
  import { getCurrentInstance } from 'vue'
  import i18n from '~/services/i18n' // example
  const app = getCurrentInstance().appContext.app
  app.use(i18n)
</script>

This method is very inconvenient, especially when using UI component libraries like Vuetify, where multiple Islands on the page require the same plugin setup. In such cases, we would need to add the following code to each Island. Even if moved outside the SFC, it's an overhead to import the same in each Island.

<script setup lang="ts">
  import { getCurrentInstance } from 'vue'
  import vuetify from '@/plugins/vuetify'
  const app = getCurrentInstance().appContext.app
  app.use(vuetify())
</script>

Describe the solution you'd like

This feature request is for a new enhanceIslands API that can enhance islands by hooking into the island app's lifecycle:

  • For Vue islands, allow app.use(plugin) for plugins like Vuetify, Pinia, etc.
  • For Preact-like apps, wrap components in a custom Provider to manage global data (i18n, themes, etc.).
  • Add any initial user state to the island.
  • Enable shared state across all islands (using a common Pinia store).
  • Support Vue/Runes/other reactivity across islands.

Unlike enhanceApp, which applies to the Vue shell, enhanceIslands would apply to all supported frameworks (e.g., Svelte, Preact, Solid). Users would need to manage conditional setup based on the framework-specific setup. Open to suggestions here 😄.

Here's one idea: we could use a simple iles attribute as a convention on the hydrated island definition, pass it to the app object, and use it within enhanceIslands to conditionally apply the required setup.

<IslandBackLink iles="back-link" href="/blog" client:load>I'm a bad example, sorry!</IslandBackLink>

The API could look like this:

// app.ts
import { defineApp } from 'iles'
import { createI18n } from '~/logic/i18n'
import { createPinia } from 'pinia'
import { createVuetify } from 'vuetify'

const pinia = createPinia({/* config */})
const vuetify = createVuetify({/* config */})

export default defineApp({
  enhanceIslands({ app }) {
    if (app.iles === "hero-widget") { // Not required when using only one framework for islands
      app.use(pinia)
      app.use(vuetify)
    }
    if (app.iles === "back-link") {
      // Some Preact setup for this island.
    }
  },
  enhanceApp({ app }) {
    const i18n = createI18n()
    app.use(i18n)
    i18n.global.locale = import.meta.env.VITE_LOCALE
  },
})

Describe alternatives you've considered

Astro had a similar API proposal (https://github.com/withastro/roadmap/blob/app-setup/proposals/0000-app-setup.md) to introduce an appEntrypoint for all supported frameworks as part of this PR: withastro/roadmap#326.

Later, Astro, in their community call stream on 4/10/22, decided to drop this idea and introduce appEntryPoint only for Vue via their Vue integration community call stream on 19/10/22 with this PR: withastro/astro#5075. In this, the user creates a _app.ts file and passes it as the appEntryPoint in their Astro Vue integration, which becomes a Vite virtual module, applying the "setup" (like vue.use plugins) for all Vue Islands.

Another related thread - withastro/roadmap#810

We already have the app.ts entry, and the new enhanceIslands can be framework-agnostic, which I believe will be simpler from a usage perspective. Moving to a "framework-specific" module would create a new "layer" in the workflow, which is what Astro follows (via their integrations API) which does have its advantages.

Additional context

This new enhanceIslands API added to the Iles core will be consumed by framework-specific createFrameworkIsland (e.g., createVueIsland) modules. For example, in: https://github.com/ElMassimo/iles/blob/main/packages/hydration/vue.ts

image

@ElMassimo ElMassimo added the enhancement New feature or request label Sep 4, 2024
@TechAkayy
Copy link
Collaborator Author

TechAkayy commented Sep 7, 2024

Given my limited knowledge of frameworks other than Vue, I did more research.

Adding plugins after the creation of the app instance is specific to Vue. For example, app.use(pinia) pattern allows for the addition of global functionality and configuration before mounting the app.

In contrast, most other frameworks (React, Preact, Solid, Svelte) use their own version of Context API and global stores to handle global injections during or within the app's initialization, rather than as a separate step.

Vue's Composition API is a better alternative, but the Vue plugin approach, due to its ease of use, is widely adopted by authors distributing component libraries, i18n, vue-router, pinia, etc. This usage is very consistent across Vue's ecosystem and is often out of the user's control when consuming these libraries.

With this in mind, it may be more appropriate and beneficial to confine the proposed enhanceIslands feature to Vue islands only. If so, may be instead of introducing a new api, the current enhanceApp itself could be extended to serve a hybrid role, both as a dev shell and to enhance Vue islands. This could potentially be achieved with a flag, where the dev shell does not have an iles property on the app instance, while the islands do.

Additionally, though not directly related to this specific feature request, considering Astro's support and release cadence for various frameworks with component structures that are JSX/TSX or have a minor flavor of JSX (e.g., Svelte, Astro), would it be more beneficial to re-strategize Iles's scope to focus solely on Vue? What are your thoughts?

@ElMassimo
Copy link
Owner

confine the proposed enhanceIslands feature to Vue islands only

I think that makes sense.

would it be more beneficial to re-strategize Iles's scope to focus solely on Vue?

Pre-rendering components from any framework is a useful feature. In practice I've only needed it for Docsearch, though. Would be interested to hear if anyone is mixing other frameworks.

I'd be ok having additional features such as enhanceVueIslands that are Vue-specific and don't have a counterpart when using other frameworks.

@TechAkayy
Copy link
Collaborator Author

Thanks Max ✌🏾. I will prepare a PR for review coming weekend 🙂!

@TechAkayy
Copy link
Collaborator Author

TechAkayy commented Sep 13, 2024

I have created a sample with two vuetify islands (screenshots at the bottom), both of them require initializing of the vuetify plugin with enhanceIslands - https://github.com/TechAkayy/pg-iles-vuetify-enhance-islands to test the above idea instead of using getCurrentInstance to add vuetify plugin within every island (redundant) like here.

Here is my app.ts, note the duplication of the plugin initializations under both enhanceApp (shell vueRenderer) and enhanceIslands, which makes me wonder if the existing enhanceApp can be extended instead of introducing a new enhanceIslands.

import { defineApp } from 'iles'
import pinia from '@/plugins/pinia'
import vuetify from '@/plugins/vuetify'

export default defineApp({
  enhanceApp({ app }) {
    app.use(pinia)
    app.use(vuetify)
  },
  enhanceIslands({ app }) {
    app.use(pinia)
    app.use(vuetify)
  },
})

I tried passing the enhanceIslands similar to props & slots via the Island.vue here and via subsequent modules - https://github.com/ElMassimo/iles/blob/main/packages/iles/src/client/app/components/Island.vue#L103. But, as enhanceIslands is a function and can't be serialized, it can't be passed to the vue island hydrator.

Also, as iles core and @islands/hydration are two separate packages, the userApp can't be imported directly like this:
import userApp from '@islands/user-app'; in hydration/vue.ts. But, hacking this directly in my sample project inside node_modules works fine (like my initial screenshot in my original post).

I have a hunch there might be a need for a hook to be exposed to achieve this across the packages. @ElMassimo, any tricks up your sleeves? This could be accomodated as part of your vite-5 PR review. Thanks! I will try again with fresh eyes later.

image

@ElMassimo
Copy link
Owner

as iles core and @islands/hydration are two separate packages, the userApp can't be imported directly

We might add the import in Island when generating the script, importing enhanceIslands from a virtual module for simplicity, and pass enhanceIslands along with the props and components, which @islands/hydration can call without knowing where it came from.

@TechAkayy
Copy link
Collaborator Author

Thanks for the insights, @ElMassimo! 😃 I finally found some time to work on the new enhanceIslands API.

I remembered that you're reviewing the Vite-5 update branch, so I decided to push my changes as a commits to my previous PR. This way, you can merge it as part of your current review that you forked. These are the two new commits 80dca90 (#274) & 650ee53 (#274). I tested it with my above Vuetify repo, and it worked perfectly (both dev & build). I didn't need to use getCurrentInstance within each island to add the Vuetify plugin.

While you're working on the Vue client hydrator, it would be great to merge @maoberlehner's PR as well: #178. Even though he raised a question about wrapping, his update successfully wraps async setup under a Suspense. I tested it too. Breaking part of an island into a child component just to wrap it in Suspense seems like unnecessary overhead, and it feels natural for users to fetch data on the client side within script setup.

It would be awesome if you could push a pre-release update (beta) so we can test everything. Let me know if there's anything else I can do to help. Again, Iles is such a simple yet powerful tool, and I’m excited to keep working with it. Thanks again for your innovative work for the community!

@TechAkayy
Copy link
Collaborator Author

Just pushed docs update for the new enhanceIslands api and also included few clarifications regarding enhanceApp as well - f9ea533 (#274). Definitely, there is room to improve, please check it out 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants