florian-lefebvre / astro-integration-kit

A package that contains utilities to help you build Astro integrations.
https://astro-integration-kit.netlify.app
MIT License
65 stars 9 forks source link

addVirtualImport should accept an array or a series of imports that can all live in one virtual export just like starlight #65

Closed jdtjenkins closed 8 months ago

florian-lefebvre commented 9 months ago

I think it's worth accepting an object or an array of objects

BryceRussell commented 9 months ago

Misunderstanding about the issue

(collapsed cause its long and unrelated)
I can try and tackle this issue, I implemented something very similar to this [here inside `astro-theme-provider`](https://github.com/BryceRussell/astro-theme-provider/blob/main/package/index.ts#L294-L466). It takes a glob, array, or object and generates a virtual module with types. Adding this functionality to AIK would give me an opportunity to make this code a lot cleaner and allow others to use it too. ## API **Questions/Things to Consider**: - Should `addVirtualImport` generate module types? If `addVirtualImport` handles the creation of imports/exports AIK could generate types for them. - If yes, there needs to be a way to buffer data/types while generating virtual modules so that it can be compiled into a string for `addDts`. Would it be beneficial to add this functionality directly to the `addDts` utility? Just now seeing that there is an issue open for this already! https://github.com/florian-lefebvre/astro-integration-kit/issues/66 - If yes, how would the `.d.ts` file be named when using the standalone utility? ### Array Modules Creating the virtual imports: ```ts addVirtualImport({ name: 'my-module/css', root: [absolute path to root/cwd for resolving relative file imports] content: ['./reset.css','./styles.css'], }) ``` Generating types for imports ```ts declare module "my-module/css" {} ``` Using import: ```ts import 'my-module/css'; ``` In the example above, the module only imports and does not include any exports. AIK could create named exports using the imported module/path but how would the `addVirtualImport` function know when to create imports or exports? To support this `addVirtualImport` could have another option like `type: 'export' | 'import'`. This use case would also allow authors to use glob patterns to generate arrays of file paths for creating virtual modules. **Example**: supporting exports in Array Modules: Creating virtual import: ```ts addVirtualImport({ name: 'my-module', type: 'export', root: [absolute path to root/cwd for resolving relative file imports], content: ['./layouts/Layout.astro', './assets/logo.png'], }) ``` Generating types for imports: ```ts declare module "my-module/layouts" { export const Layout: typeof import("[...]/layouts/Layout.astro").default; } declare module "my-module/assets" { export const logo: typeof import("astro").ImageMetadata; } ``` Using imports: ```ts import { Layout } from 'my-module/layouts'; import { logo } from 'my-module/assets'; ``` #### Things to Consider - `addVirtualImport` does not know the named exports of the file your importing meaning all imports inside Array modules would have to have a `default` export - How would the virtual module be named? My assumption is that the `name` acts like a 'base' and the nested structure of the import is preserved inside the module name (`['./src/layouts/Layout.astro']` would become `import { Layout } from 'my-module/src/layouts'`) - Named exports are created using the name of the file, this means invalid variable names like `logo-dark` would have to be camel cased to `logoDark` - Instead of a option like `type: 'import' | 'export'`, it could be something like `export: [ 'astro' ], import: [ 'css' ]` to define exactly what file types get exported/imported ### Object Modules Object modules would have more control over the name of the module/named exports, and you can add a default export ```ts addVirtualImport({ name: 'my-module', type: 'export', root: [absolute path to root/cwd for resolving relative file imports], content: { default: './config.ts' layouts: './layouts/Layout.astro', assets: { default: './logo-light.png', light: './logo-light.png', dark: './logo-dark.png' } } }) ``` Generating types for imports: ```ts declare module "my-module" { const value: typeof import("[...]/config.ts").default; export default value; } declare module "my-module/components" { export const Button: typeof import("[...]/layouts/Layout.astro").default; } declare module "my-module/assets" { export const light: typeof import("astro").ImageMetadata; export const dark: typeof import("astro").ImageMetadata; export default light; } ``` Using imports: ```ts import { Layout } from 'my-module/layouts'; import { Button } from 'my-module/components'; import config from "my-module"; ``` #### Things to consider The example above only takes into consideration exporting but does not handle cases where you just want to import (ex: `import 'css/styles.css'`) --- ### Let me know what you think There is a lot of info here and I am sure I missed some details, but let me know what you think the API and how it could be improved!
florian-lefebvre commented 9 months ago

I'm not sure I fully understand your suggestion. I think it's great to keep addVirtualImport as a low-level primitive, but what you're describing could be another utility/plugin built on top of it! How should such utility be called?

BryceRussell commented 9 months ago

Maybe I misunderstood the issue? I was trying to answer the question "What would it look like if addVirtualImport accepted an array or object for creating virtual modules". I agree this API is too complicated for a lower level primitive like addVirtualImport (especially if it included typegen) so it would make more sense if this was a higher level utility.

The basic idea for this API is that it can be annoying to have to compile your modules into strings so being able to create a module from an array or object makes it more user friendly and opens the door for doing stuff like typegen for modules and resolving relative imports with a custom base/root. Maybe something like this would be a better API:

generateVirtualModule({
  name: 'my-module',
  root: integrationDir,
  import: [ './styles.css' ],
  export: {
    default: JSON.stringify(config),
    Button: './components/Button.astro' 
  },
})

If the higher level nature of this API isn't suited for AIK no worries! Let me know and I can always make this a separate package

florian-lefebvre commented 9 months ago

Yeah the issue is originally about something way simpler than your proposal and we'll keep it that way! However, I'd love to see an AIK plugin for this! You can either make it 3rd party or a PR, and we can discuss on that one directly! I'm sure Jacob and Luiz will have some valuable feedback as well

BryceRussell commented 9 months ago

Sounds good, sorry for the misunderstanding! I thought that maybe I could help out with this issue since it has the "help wanted" and "good first issue" tags but there is not a lot of information about what the goal of the issue is. Do you mind explaining a bit more about what this issue is about? The title: "array or a series of imports that can all live in one virtual export just like starlight" sounds like it is describing starlight's virtual:starlight/user-css virtual module that has a series of CSS imports inside a single virtual module like:

import '../styles.css';
import '../styles.css';
import '../styles.css';
import 'virtual:starlight/user-css';
florian-lefebvre commented 9 months ago

Yeah this is clearly our fault for not writing stuff down! Currently, calling addVirtualImport creates one vite plugin per virtual import (see https://github.com/florian-lefebvre/astro-integration-kit/blob/main/package/src/utilities/add-virtual-import.ts#L10). What we want is allow it to receive an array instead and do things similar to this https://github.com/withastro/starlight/blob/main/packages/starlight/integrations/virtual-user-config.ts#L11.

I think it could look like this in terms of API:

// utility
addVirtualImports({
    updateConfig,
    imports: {
        "virtual:whatever": "export const ..."
    }
})

// plugin
addVirtualImports({
    "virtual:whatever": "export const ..."
})

That's a breaking change because that would mean changing both the type signature and the utility name. I'm not concerned about breaking changes tho because we're in an experimental phase, so that just needs to be a minor!

If you have other/better ideas for the API, feel free to just do it and we'll review on the PR!

BryceRussell commented 9 months ago

That makes a lot more sense 😅 I will start exploring this and open a PR when I have something working, thanks!

florian-lefebvre commented 9 months ago

Awesome thank you! Let me know if you need anything

jdtjenkins commented 9 months ago

@BryceRussell That was such a detailed an well thought out explanation! This is completely my fault because I kinda just yeeted this issue out with literally 0 context!

Yes, for this particular thing I was just thinking exactly what @florian-lefebvre said! But mate. That proposal is FUCKING SICK and I love it. Yes. I love it, especially as I'm making themes now, this would be absolutely perfect for me if I'm honest. And I think that kind of high-level abstraction is perfect for an AIK plugin absolutely.

BryceRussell commented 9 months ago

@florian-lefebvre or @jdtjenkins In this example:

// utility
addVirtualImports({
    updateConfig,
    imports: {
        "virtual:whatever": "export const ..."
    }
})

There is no name property, is that on purpose? If so, how should the Vite plugin that resolves these imports be named? I could use the name/key of the first import inside imports to create the plugin name but that doesn't seem right?

florian-lefebvre commented 9 months ago

mmmh you're right! I think passing a name makes sense then!

Fryuni commented 8 months ago

I think the Vite plugin could be the name of the integration, with maybe a count if it is called multiple times.

I'll have a look in the PR.

But @BryceRussell, I loved your idea for a new plugin. It seems like a lovely middle ground between a combination of AIK's addVirtualImport and addDts and my own inlineModule from Inox Tools. Might be a more friendly API for those who don't want to generate everything manually but also don't want the level of magic from Inox Tools.