addVirtualImport should accept an array or a series of imports that can all live in one virtual export just like starlight
jdtjenkins opened this issue ยท 13 comments
I think it's worth accepting an object or an array of objects
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
. 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? IfaddVirtualImport
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 theaddDts
utility? Just now seeing that there is an issue open for this already! #66 - If yes, how would the
.d.ts
file be named when using the standalone utility?
- 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
Array Modules
Creating the virtual imports:
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
declare module "my-module/css" {}
Using import:
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:
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:
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:
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 adefault
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 becomeimport { 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 tologoDark
- Instead of a option like
type: 'import' | 'export'
, it could be something likeexport: [ '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
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:
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:
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!
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?
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
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
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';
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!
That makes a lot more sense ๐ I will start exploring this and open a PR when I have something working, thanks!
Awesome thank you! Let me know if you need anything
@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.
@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?
mmmh you're right! I think passing a name makes sense then!
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.