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

🐛 BUG: astro-embed integration fails due to .astro file extension error #4071

Closed
1 task
scottaw66 opened this issue Jul 27, 2022 · 9 comments · Fixed by #4073 or #4623
Closed
1 task

🐛 BUG: astro-embed integration fails due to .astro file extension error #4071

scottaw66 opened this issue Jul 27, 2022 · 9 comments · Fixed by #4073 or #4623
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@scottaw66
Copy link
Contributor

scottaw66 commented Jul 27, 2022

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

  • I am willing to submit a pull request for this issue.

Screen Shot 2022-07-27 at 8 10 50 AM

@github-actions github-actions bot added this to Needs Triage in 🐛 Bug Tracker Jul 27, 2022
🐛 Bug Tracker automation moved this from Needs Triage to Done Jul 27, 2022
@delucis delucis reopened this Jul 27, 2022
🐛 Bug Tracker automation moved this from Done to Needs Triage Jul 27, 2022
@matthewp
Copy link
Contributor

matthewp commented Jul 27, 2022

Thanks, the logic for how we determine to mark an ecosystem package to be automatically marked as noExternal is here:

// Scans `projectRoot` for third-party Astro packages that could export an `.astro` file

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

@matthewp matthewp added - P4: important Violate documented behavior or significantly impacts performance (priority) s1-small labels Jul 27, 2022
@Remcoman
Copy link

Remcoman commented Aug 1, 2022

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
Copy link
Member

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
Copy link

taoeffect commented Sep 4, 2022

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
Copy link
Member

delucis commented Sep 4, 2022

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 #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!

🐛 Bug Tracker automation moved this from Needs Triage to Done Sep 6, 2022
@taoeffect
Copy link

taoeffect commented Sep 6, 2022

@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
Copy link
Member

delucis commented Sep 6, 2022

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
Copy link

@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
Copy link
Member

delucis commented Sep 6, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
No open projects
8 participants