withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
46.65k stars 2.48k forks source link

πŸ› BUG: astro-embed integration fails due to .astro file extension error #4071

Closed scottaw66 closed 2 years ago

scottaw66 commented 2 years ago

What version of astro are you using?

1.0.0-rc.1

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Mac (macOS Monterey 12.4)

Describe the Bug

Site start in dev mode fails with astro-embed integration enabled in astro.config.mjs with warning about unknown .astro file extension type.

[plugin:astro] Unknown file extension ".astro" for /Volumes/BFD/Sites/astrobug/node_modules/@astro-community/astro-embed-twitter/Tweet.astro

You likely need to add this package to `vite.ssr.noExternal` in your astro config file.

Link to Minimal Reproducible Example

https://github.com/scottaw66/astrobug

Participation

matthewp commented 2 years ago

Thanks, the logic for how we determine to mark an ecosystem package to be automatically marked as noExternal is here: https://github.com/withastro/astro/blob/13b4f8ad887d0d4e8efbf9f74185432f9cdf264e/packages/astro/src/core/create-vite.ts#L168

If anyone wants to take a look at why this one is not being marked, that's the place to go.

Remcoman commented 2 years ago

It seems like the noExternal option does not contain the @astro-community/astro-embed-integration dependency because it's referenced from astro-embed. The logic for getAstroPackages only looks 1 level deep.

I was able to fix this by adding the /@astro-/ regex to the noExternal array. However I'm not sure if this is the best solution. Any other ideas?

FredKSchott commented 2 years ago

Quick fix: add these packages to our internal noExternal array. Longer-term fix: What is the maintenance story of astro-embed? Is this officially maintained or not?

taoeffect commented 2 years ago

Hi, I couldn't get the integration to work with .md files, the links and embeds just disappear from the document. There is no embed and the tweets and youtube embed links are no longer there either. Yes, I did run with the special flag: npm run dev --experimental-integrations

Here's my astro.config.mjs file:

import { defineConfig } from 'astro/config'
import mdx from '@astrojs/mdx'
import sitemap from '@astrojs/sitemap'

// Use: npm run dev/build --experimental-integrations
import embeds from 'astro-embed/integration'

// https://astro.build/config
export default defineConfig({
  site: 'https://example.com',
  integrations: [embeds(), mdx(), sitemap()],
  vite: {
    ssr: {
      noExternal: [/@astro-/]
    }
  }
})
delucis commented 2 years ago

Spent some time investigating what we can do here today.

Looking for a quick fix?

Add /@astro-community/ or a similar RegExp pattern to vite.ssr.noExternal in your Astro config file as suggested by @Remcoman above:

import { defineConfig } from 'astro/config';

export default defineConfig({
  vite: {
    ssr: {
      noExternal: [/@astro-community/]
    }
  }
})

More long-term solutions

Background

Third-party packages that provide .astro files need to be processed by Vite. This is why adding them to noExternal fixes this bug. Astro includes some heuristics added in https://github.com/withastro/astro/pull/2665 to try to auto-add third-party Astro packages to noExternal.

These heuristics fail for astro-embed because it is a collection of several sub-packages:

astro-embed
β”œβ”€β”€ @astro-community/astro-embed-integration
β”œβ”€β”€ @astro-community/astro-embed-twitter     <-- contains .astro
└── @astro-community/astro-embed-youtube     <-- contains .astro

The current heuristics only check direct dependencies and add them to noExternal. In the above example, astro-embed gets included in noExternal but none of its dependencies do, causing them to fail.

Possible solutions

1 β€” Userland: Never depend on another Astro package from your third-party package

I could refactor astro-embed to be a single package. This would solve this immediate issue.

However, this is still brittle. Let’s say someone builds a theme utility package that provides layouts you can import and they decide to use astro-embed (or another package) in those layouts. The same bug would resurface because the dependency chain is [Astro project] -> cool-astro-layouts -> astro-embed.

2 β€” Astro: Add some RegExps to Astro’s noExternal defaults

One of the heuristics we currently use is: does the package name start with astro-.

Could we add /(astro-|-astro)/ or something similar to noExternal by default? Still brittle, but we are already doing something like this and this could cover indirect dependencies.

3 β€” Astro: run our heuristics against all dependencies

This is notably NOT an option! Running heuristics for all project dependencies would be a performance disaster. docs for example has >1,000 dependencies. We can’t risk loading 1,000+ package.json files on server startup πŸ˜…

4 β€” Astro: run heuristics on slightly more dependencies

We could modestly expand our current heuristics so that if we decide a direct dependency is an Astro package, we then run the heuristics for its direct dependencies (and so on recursively):

astro-package <── mark as noExternal
β”œβ”€β”€ normal-dep ──x stop checking dependencies
└── astro-dep <── mark as noExternal
    └── normal-dep ──x stop checking dependencies

Obviously this would be slightly more expensive than what we currently do, but should be fairly robust: it’s unlikely for a package we decide is not an Astro package, to have an Astro package dependency.

5 β€” Vite Expert’s Delight?

Is there another way entirely to ensure Vite processes third-party .astro files? I’m not experienced enough to know if there are solutions outside of the noExternal model already established.


If I don’t hear anything else, I’ll take a stab at 4 above as it seems most promising to me!

taoeffect commented 2 years ago

@delucis Thank you for the reply and for the detailed explanation!

I think however I'm running into a separate issue, specially with the integration part of the @asto-embed plugin as described here: https://github.com/astro-community/astro-embed/tree/main/packages/astro-embed#using-the-integration

So while using your "quickfix" above I get no error, I also get no functionality, still (read on to see how I got things working with a different method!):

Here's my configuration with the quickfix:

import { defineConfig } from 'astro/config'
import mdx from '@astrojs/mdx'
import sitemap from '@astrojs/sitemap'
import remarkBreaks from 'remark-breaks'

// Use: npm run dev/build --experimental-integrations
import embeds from 'astro-embed/integration'

// tables
import remarkGfm from 'remark-gfm'

export default defineConfig({
  site: 'https://blog.okturtles.org',
  markdown: {
    remarkPlugins: [remarkGfm, remarkBreaks],
  },
  integrations: [embeds(), mdx(), sitemap()],
  vite: {
    ssr: {
      noExternal: [/@astro-community/]
    }
  }
})

With this method, no embeds are shown at all. In other words, when a markdown file contains a line that looks like this:

Lorem ipsum text.

https://www.youtube.com/watch?v=XXXXX

Foo bar baz.

The link is stripped from the generated content, and no embed is visible. The same occurs with Twitter links.

However, using a specific remark plugin, I was able to get better results, with actual embeds being generated:

import { defineConfig } from 'astro/config'
import mdx from '@astrojs/mdx'
import sitemap from '@astrojs/sitemap'
import remarkBreaks from 'remark-breaks'

// embed via remark
import remarkEmbedder from '@remark-embedder/core'
import oembedTransformer from '@remark-embedder/transformer-oembed'

const remarkEmbedPlugin = [remarkEmbedder.default, {
  transformers: [oembedTransformer.default],
  // https://github.com/remark-embedder/transformer-oembed/issues/25#issuecomment-888613740
  // https://github.com/remark-embedder/core#handleerror-errorinfo-errorinfo--gottenhtml--promisegottenhtml
  handleError ({error, url, transformer}) {
    if (transformer.name !== '@remark-embedder/transformer-oembed' || !url.includes('twitter.com')) {
      // we're only handling errors from this specific transformer and the twitter URL
      // so we'll rethrow errors from any other transformer/url
      throw error
    }
    return `<p style="color:red">ERROR: Unable to embed <a href="${url}">this tweet</a> (possibly deleted).</p>`
  }
}]

// tables
import remarkGfm from 'remark-gfm'

export default defineConfig({
  site: 'https://blog.okturtles.org',
  markdown: {
    remarkPlugins: [remarkEmbedPlugin, remarkGfm, remarkBreaks],
  },
  integrations: [sitemap(), mdx()],
})

So this other method works the way the integrations section for 'astro-embed/integration' is described to work, but it actually works.

delucis commented 2 years ago

Hi @taoeffect β€” that's a sightly different issue concerning MDX support. Haven't done any work on astro-embed since Astro launched its MDX integration so the embed integration is still only targeting a legacy components in .md mode.

Would you mind opening an issue for MDX support in the astro-embed repo?

taoeffect commented 2 years ago

@delucis Perhaps I'm misunderstanding, but we're not using .mdx files in our blog.

These are regular .md files containing twitter and youtube links on their own lines that we'd like converted to embeds.

The README for the astro-embed project even contains an example:

I saw this cool Tweet the other day:

https://twitter.com/astrodotbuild/status/1511750228428435457

This is exactly what we have too, but it doesn't work when using 'astro-embed/integration'.

delucis commented 2 years ago

That integration was built for a previous version of Astro that supported components in .md files, so that's why it's not working for you. Either way, feel free to open issues on the astro-embed repo as I suspect discussion on this closed unrelated issue risks just getting lost.