ElMassimo / iles

🏝 The joyful site generator
https://iles.pages.dev
MIT License
1.07k stars 31 forks source link

Should display an error when useDocuments is used with a variable #146

Open tex0l opened 2 years ago

tex0l commented 2 years ago

Hi!

I stumbled upon an issue I couldn't explain. When I run npm run dev in a dummy app with:

<script setup lang="ts">
const foobar = '~/pages'
console.log(useDocuments(foobar).value.length) // it always logs 0
console.log(useDocuments('~/pages').value.length) // it logs 3 (in my case)
</script>

When looking at the transpiled code in my browser, I saw:

import _documents_0 from '/@islands/documents?pattern=ooba';
import _documents_1 from '/@islands/documents?pattern=~/pages'

I could find why this has this behaviour: https://github.com/ElMassimo/iles/blob/80eb10366ddefea217f32d138b8b79e1aea5366e/packages/iles/src/node/plugin/documents.ts#L134

I am deeply troubled by this static analysis of the code, which fails when using useDocuments with a variable. Is it a usual pattern, or a design choice from your end? This seems really flaky to me, and needs at the very least to be documented.

(otherwise, iles is nice :) thanks for the great work)

tex0l commented 2 years ago

The reason why I stumbled upon this is because I want to build a wrapper to order the Documents that looks like:

import { ref, computed } from 'vue'
import { useDocuments } from 'iles/dist/client'

export const defaultCompareFn = (x: any, y: any) => x.order - y.order // this is a convention I use to sort my pages in the menu

export const dateCompareFn = (x: any, y: any) => x.date - y.date 

export default (globPattern: string, compareFn = defaultCompareFn) => computed(() => useDocuments(
  globPattern).value.sort(compareFn))

I would have liked to re-use this composable for my navbar and my blog posts.

ElMassimo commented 2 years ago

Thanks for reporting!

Generating imports requires static analysis, there's no way to support a dynamic value, as Rollup needs to know which files to process at compile time.

It's very similar to glob imports in Vite:

You should also be aware that glob imports do not accept variables, you need to directly pass the string pattern.

This would be a nice addition to the docs, pull requests are welcome.


At the moment it seems that quotes in the useDocuments call are treated as optional, so the expression is matched.

Ideally, îles should throw an error on analysis, just like Vite would if you used import.meta.globEager with a variable instead of a literal.

That way users that weren't aware of this limitation in build systems will see an explicit error instead of unexpected behavior.

Pull requests for this are also welcome 😃

tex0l commented 2 years ago

All right, thanks for the swift reply. This is my first project with Vite!

ElMassimo commented 2 years ago

Documented here:

Screen Shot 2022-06-30 at 13 19 40