antfu / vite-plugin-md

Markdown with Vue for Vite
MIT License
606 stars 89 forks source link

[Bug]: When I upgraded to version `0.12.x` I couldn't get the markdown `formatter` #64

Closed nonzzz closed 2 years ago

nonzzz commented 2 years ago

Description

In ver 0.11.7

When i use dynamic import load .md formatter info. i can got them.

image

This is my chrome console

image

After ver 0.11.7 in new version 0.12.4

In my console

image

@antfu PTAL 🙏

yankeeinlondon commented 2 years ago

thanks for bringing this to my attention ... i've a unit test now which shows this failing (I don't personally use this atm) and will see what I can do to get this fixed soon.

nonzzz commented 2 years ago

thanks for bringing this to my attention ... i've a unit test now which shows this failing (I don't personally use this atm) and will see what I can do to get this fixed soon.

thanks, :)

yankeeinlondon commented 2 years ago

if you saw some other comments ... I appologize as I was thinking that your issue was related to a different issue regarding importing frontmatter properties. They may indeed be related but I think I'm going to need more information from you.

What would be best is if you can create a minimally reproducible example of this on StackBlitz playground. Also when you say "formatter info" what are you referring to? Your example seems to revolve around frontmatter metadata and I do wonder if there was an undocumented change in version 0.12.x (in fact it was probably 0.11.8) which caused this.

If I'm right in my theory then there has been no loss in utility but there was change in a semi-formal part of the API contract. Let me illustrate with an example:

yankeeinlondon commented 2 years ago

here's an example of what the markdown files from version 0.12.x are converted to:


<template><div class="markdown-body"><h1>{{title}}</h1>
<p>A nice image to brighten your day:</p>
<p><img src="%7B%7Bimage%7D%7D" alt="test image"></p>
</div>
</template>
<script setup>const frontmatter = {"image":"/images/foobar.jpg","title":"Frontmatter in Action"}
 const image = "/images/foobar.jpg"
 const title = "Frontmatter in Action"
 const excerpt = undefined
defineExpose({ frontmatter })
const head = {"image":"/images/foobar.jpg","title":"Frontmatter in Action"}
</script>
yankeeinlondon commented 2 years ago

Does what I'm describing make sense and does it map to the issue you're having?

nonzzz commented 2 years ago

Does what I'm describing make sense and does it map to the issue you're having?

yeah, And i will take a minimally exmaple :)

nonzzz commented 2 years ago

Hi, Here is example.

See the App.vue , you can see the frontmatter meta info in the viewport.

yankeeinlondon commented 2 years ago

fantasic reproduction, thanks @XeryYue . WIll look over now and hopefully get this turned around. It dips into functionality I had definitely not built tests around and not personally used but excited to get this sorted and get some tests around it so it won't regress in the future.

yankeeinlondon commented 2 years ago

@XeryYue @antfu might need your collective brain power on how best to create a good unit test for this. I figured that this test:

describe('validate that imports of frontmatter are possible', () => {
  it('title is found', async() => {
    const md = readFileSync('test/fixtures/my-markdown.md', 'utf-8')
    const comp = markdownToVue('my-markdown.md', md)
    const imp = (await import('test/fixtures/my-markdown.md')) as { default: Component; title: string }
    expect(imp.default).toBeDefined()
    expect(imp.title).toBeDefined()
  })
})

along with this shim file:

declare module '*.md' {
  import { Component } from 'vue'
  const Component: Component
  export const frontmatter: Record<string, unknown>
  export const title: string
  export default Component
}

Should be just fine but when I do this using 0.11.7 I still get this error:

Error: Failed to parse source for import analysis because the content contains invalid JS syntax. You may need to install appropriate plugins to handle the .md file format.

Which at first I presumed to be because the library actually uses TSUP to build not ViteJS and so I presume Vitest wasn't able to reach this plugin's support for MD files. I thought i'd corrected this by putting the the following vite config file in the root of the directory but I still get the same error:

/// <reference types="vitest" />
import { defineConfig } from 'vite'
import Markdown from './src/index'

// used for testing, library code uses TSUP to build exports
export default defineConfig(() => ({
  test: {
    dir: 'test',
  },
  plugins: [
    Markdown(),
  ],
}))

I'll continue to troubleshoot but would be nice to have a strong feedback loop.

yankeeinlondon commented 2 years ago

I believe I should have a fix for this soon but still keen on getting a more direct test for this so if either of you have any ideas how to test the import from node land I'd appreciate your help.

Note: in the mean time I can create a test for this just looking for string patterns but I'd assume that people would more likely want to import in their node code most times rather than relying on imports inside the browser.

yankeeinlondon commented 2 years ago

I have submitted a fix which should make this work.

Note, however, the original implementation was not done quite to the specification it suggested in docs. Meaning ... the frontmatter props were exported as root level named exports on the module. This is undesirable because it can cause namespace collisions and because it makes typing the bundle more troublesome with shims. It also is inconsistent with the way defineExpose() presents these properties.

PR

image

A markdown file now is converted to a structure like this:

<script setup>
  defineExpose({ frontmatter: {"title":"Hello World","description":"testing is the path to true happiness"} })

 const title = "Hello World"
 const description = "testing is the path to true happiness"
 const excerpt = undefined
</script>

<script>
  export const frontmatter = {"title":"Hello World","description":"testing is the path to true happiness"}
</script>
yankeeinlondon commented 2 years ago

ok @antfu has released it now as v0.13.0; let me know if it fixes your issues. On my modification of your example code it appears to work: https://stackblitz.com/edit/vitejs-vite-lgeh8d-n1sz3c?file=package.json

@antfu I'm still concerned that we can't import the markdown files in NodeJS (that's true in 11.x as well as 12/13.x) and to make better unit tests I'd need to do this. I remember reading that vitest can operate in Vite and non-Vite environements. I added a vite.config.ts file to get vitest to run with ViteJS but maybe this isn't happening? We still get this error: image

  it('can import a markdown file from the filesystem', async() => {
    const c = await import('./fixtures/simple.md')
    expect(c).toBeDefined()
  })
nonzzz commented 2 years ago

@yankeeinlondon It's back to work again. Thank you very much : )