Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

question: expected behavior for links from/to locations not in a current content collection #26

Open
techfg opened this issue Apr 11, 2024 · 3 comments

Comments

@techfg
Copy link
Contributor

techfg commented Apr 11, 2024

Currently, the plugin runs against ANY markdown file, even if the file exists outside of a content collection. However, there is logic in the plugin that assumes the file its processing is within a content collection. This yields three scenarios that could lead to issues:

  1. Files within a content collection can reference files outside of its own content collection but within contentDir in another content collection. For example, src/content/posts/post-1.md could contain a markdown link to ../newsletter/post-1.md (a different content collection). Currently, this transformation works IF the actual page path for the site to the newsletter post-1 page includes the physical path name of newsletter (e.g., /newsletter/post-1).

  2. Files within a content collection can reference files outside of the content directory entirely. For example, src/content/posts/post-1.md could contain a markdown link to ../../pages/page-1.md. Currently, this transformation fails because it transforms to /../pages/page-1 which is not a valid path.

  3. Files outside of the content collection can reference files inside or outside of a content collection. For example, /pages/page-1.md could contain a markdown link to ../content/posts/post-1.md. This scenario will fail because the transformed url will become /pages/post-1

I think there is something to be said for allowing the portability of file references to markdown files in/out/across pages, content collections, etc., however the current logic does not support it but does actually do a transformation.

The question on the table is what scenarios to support for file references to {.md,.mdx} files:

  1. In/Around a single content collection only
  2. In/Around any content collection within contentDir
  3. Anywhere and everywhere

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

In the repro, there are two "Link" sections on each page to help navigate since a lot of links do not work because of the attempted transformations described above.

MD Links - This section always points to the corresponding .md file (e.g., ./page-1.md)
Explicit Links - This section always points to the site relative root page (e.g., /page-1)

Steps to reproduce:

  1. Open repro
  2. In the MD Links section, click any of the links starting with Root or Subdir

Expected Result:
This is the question since this page is /pages/index.md and not in a content collection

Actual Result:
Transformed url is based at /pages and all 404

  1. Click Newsletter 1 or Post 1

Expected Result:
That is the question

Actual Result:
Because of the physical directory structure is the same as the site url structure for the collection, the links actually work but if the directory structures were different, they would not

  1. Bounce around the pages across /pages/* and /newsletter/* and /posts/*. They all contain the same set of links separated with MD Links and Explicit Links. All the explicit links work but only some of the MD Links work based on physical directory structure, etc.

My thought is that for v1, only links in/around a single content collection should be supported and everything else should fail the IsValidRelativeLink test. This would be consistent with how Astro treats content collections generally speaking.

See #27 as well since its somewhat related although still separate and distinct.

Thoughts?

@techfg techfg changed the title bug (or intended): links outside of content collections are transformed question: expected behavior for links from/to locations not in the current content collection Apr 28, 2024
@techfg techfg changed the title question: expected behavior for links from/to locations not in the current content collection question: expected behavior for links from/to locations not in a current content collection Apr 28, 2024
@techfg
Copy link
Contributor Author

techfg commented Apr 28, 2024

Changed this issue from "bug" to "question" after creating issue #57 which covers the specific situation regarding links to markdown outside of a content collection since current behavior to those will result in incorrect links being transformed.

The scenario and questions raised in this issue's OP still stands, however. Assuming PR #49 is merged, and preferrably #50 is merged and #53 implemented, links from anywhere to a markdown file within a content collection directory should be reliable. This means that question 1 & part of question 3 (outside collection dir linking to inside a collection dir) in this issue's OP are addressed - again, relies on at least #49 being merged to cover basic scenarios while PR #50 & addressing Issue #53 would provide the flexibility needed to support all known scenarios.

Repositioning this issue to clearly define expected behavior in all scenarios so that any remaining issues can be fixed and/or required gaps/features backlogged.

The questions are, what should occur in the following scenarios:

  1. Markdown file in content collection directory links to a markdown file inside the same content collection directory (currently supported)
  2. Markdown file in content collection directory links to markdown file in another content collection directory (unofficially supported but could become official with feat: Add srcDir & align collection base on a per collection basis (remove contentPath + contentPathMode) #49 and preferably feat: add support for specifying collection names used in transformed URL paths #50 & feat: add support for dynamically resolving collection name and/or url path #53)
  3. Markdown file in a content collection directory links to a markdown file outside of a content collection directory (would require a new feature)
  4. Markdown file outside of a content collection directory links to a markdown file inside a content collection directory (unofficially supported but could become official with feat: Add srcDir & align collection base on a per collection basis (remove contentPath + contentPathMode) #49 and preferably feat: add support for specifying collection names used in transformed URL paths #50 & feat: add support for dynamically resolving collection name and/or url path #53)
  5. Markdown file outside of a content collection directory links to a markdown file outside a content collection directory (would require a new feature)

@techfg
Copy link
Contributor Author

techfg commented Dec 13, 2024

@vernak2539 -

I think reaching a decision on this one, especially given Astro v5 allowing content collections to live anywhere (see #74) is becoming more pressing.

My thoughts, for now, are that the plugin only resolve relative references within the "current" collection directory. Any relative links to another collection or anywhere else would be skipped. Currently, #58 ensured that the link must be in a content collection directory but I think we take it step further and restrict to the "current" content collection directory.

Although it could be done and likely done reliably, I think there are just too many possibilities and until there is stability with v5 and to limit potential issues, I think we still to "current collection" only. This would mean the answers to the questions I posed in my last comment are:

  1. Supported
  2. Not Supported
  3. Not Supported
  4. Not Supported
  5. Not Supported

Let me know your thoughts.

@vernak2539
Copy link
Owner

Sorry! I'll try to look at this one when I can (I've gotten through a few others though!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants