vernak2539 / astro-rehype-relative-markdown-links

Rehype Plugin for Astro that allows for usage of relative links between markdown files
https://www.npmjs.com/package/astro-rehype-relative-markdown-links
15 stars 4 forks source link

feat: support mapping a content collection directory name to a site page path #24

Open techfg opened 7 months ago

techfg commented 7 months ago

In a situation where a custom collection in a directory of src/content/docs is mapped to something other than the physical content collection directory name, the wrong url is generated.

It is mentioned in the assumptions located at the top of the README that this scenario is not covered, however I think it would be fairly simple to, at a minimum, handle collection name mappings (path -> directory). Beyond that, for things like handling other path segments, it gets way more complex without internal Astro info and/or metadata in options.

For example, in the following directory structure, the [...slug].astro is written to use the content collection docs in the directory docs, however the path to the page (virtualized essentially) is my-docs/test and not docs/test.

.
├── src/
│   ├── content/
│   │   ├── docs/
│   │   |   └── test.md
|    ├── pages/
│   │   ├── my-docs/
│   │   |   └── [...slug].astro

Repro: https://stackblitz.com/edit/github-pdzrjo-gg4trc

Steps to reproduce:

  1. Open repro
  2. Click on any of the links in the Collection Root or Collection Subdir sections

Expected Result: The links generated should be my-docs/[slug] and navigate successfully

Actual Result: The links generated are docs/[slug] and 404

  1. Go to the Debug page
  2. Click any of the links and they will all work because they target my-docs/[slug]

We do not have internal information so there is no current way to detect this situation. Ideas on ways to solve for this situation:

  1. Add an option collectionNames which is a key/value pair that makes a physical directory to the collection name. For example, using the above:

    export default defineConfig({
    markdown: {
    rehypePlugins: [
      [
        rehypeAstroRelativeMarkdownLinks,
        {
          collectionNames: {
            docs: 'my-docs`
          }
        },
      ],
    ],
    },
    });
  2. Leverage metadata in the link in the markdown (similar to the way classes, titles, etc. work)

    [Some Text](some-url title){ .class-1 }

I think Option 1 makes the most sense as Option 2 is highly dependent on markdown processer, is very error prone and would require every link to have the metadata rather than just a single place to do the mapping as would be the case in option 1.

Thoughts? Other ideas on how to solution and if you think this scenario is worth solving for?

vernak2539 commented 7 months ago

This is actually a really good idea! I really like "Option 1" as an opt-in. Let me get through the other PRs/Issues that you've created to enable this

vernak2539 commented 20 hours ago

@techfg just coming back to this. I think this is really a good idea and creates a lot of flexibility (yeah, repeat of the comment back in April ha)

I'm a bit confused though as to the order in which you see things getting merged (too many issues / prs floating around haha - it's a good thing). I think I'm understanding, but just want to confirm.

50 contains all the changes to enable this PR (i.e. #49 and further changes). Then, #49 could be closed. Is that correct? Or do you think #49 should be merged first then #50 will be easier to reason about and should be merged after?

techfg commented 18 hours ago

Hi @vernak2539 -

Sorry for the confusion. Technically speaking #50 contains everything in #49 and therefore #49 could be closed and only #50 merged.

That said, the PRs to deviate a little in terms of commits beyond a certain point (where I went back and fixed a few things in #49) and also there are references to #49 in various issues in the repro so in order to keep things straighforward for future reference, I think it makes sense to merge #49 and then merge #50. I don't think there will be, but if any conflicts arise in #50 after merging #49 I'll be happy to resolve.

Thanks!