astro-icon icon indicating copy to clipboard operation
astro-icon copied to clipboard

`svgo` and SSR

Open JuanM04 opened this issue 2 years ago • 14 comments

svgo is a Node.js-only package, thus, it doesn't work in the brand-new environments that Astro SSR supports (e.g. Netlify/Vercel Edge, Deno, Cloudflare workers).

Is there any other option that supports more environments? Or there could be a way to disable svgo altogether so Astro can build successfully?

JuanM04 avatar May 06 '22 20:05 JuanM04

SVGO and SSR

To add more context to the issue, the error that is thrown when building is this:

22:48:08 [build] Building SSR entrypoints...
[commonjs] Cannot bundle Node.js built-in "os" imported from "node_modules/svgo/lib/svgo-node.js".
Consider disabling ssr.noExternal or remove the built-in dependency.

Many of the SSR adapters enable noExternal in the vite ssr configuration (for good reason). I've tried to remove that by patching the integration module, but that just results in other errors:

 error   Build failed with 4 errors:
  node_modules/svgo/lib/svgo-node.js:3:19: ERROR: Could not resolve "os"
  node_modules/svgo/lib/svgo-node.js:4:19: ERROR: Could not resolve "fs"
  node_modules/svgo/lib/svgo-node.js:5:34: ERROR: Could not resolve "url"
  node_modules/svgo/lib/svgo-node.js:6:21: ERROR: Could not resolve "path"

Fetching SVGs and SSR

Another issue with SSR is that the default behaviour of astro-icon is to fetch the icon via the web service. This makes sense when building a static site, but in SSR mode it would mean we make requests to the web service all the time, which I don't think makes sense. (I could be wrong about all this though, this is just what I gathered from the source code.)

I'm not sure how to fix all of this. It seems like the perfect solution would be to make the SVG fetching and optimisation happen at the build step instead of at runtime, but looking at the Astro Integration API docs I'm not sure we can hook into that and change the source code during build? @natemoo-re If this is in fact possible though, I'm open to do a PR for it.

JReinhold avatar Jun 20 '22 21:06 JReinhold

I have the exact same issue. This feature is necessary because many people run astro with ssr in mind and it makes that impossible when using astro-icon

ubarilan avatar Jul 22 '22 19:07 ubarilan

Having the same issue when trying to use with Cloudflare Pages

jjjrmy avatar Aug 19 '22 17:08 jjjrmy

Resolution / update badly needed. Cannot deploy to CF pages using SSR :-(

jacobsandersen avatar Aug 22 '22 07:08 jacobsandersen

After doing some research, I have found that svgo actually includes a browser version as well as a node version. I have feebly tried to fork and create a PR but I cannot get it working properly on my dev server. Node complains about unexpected exports, even though a diff between the versions shows only some work in the optimize function...

Sigh.

Anyway, one can import optimize from svgo for the node-js version or one can import optimize from svgo/dist/svgo.browser for the browser version. Can someone better than I possibly capitalize on this to help get this project in the proper direction for ssr?

My sad attempt is here, and for whatever reason I can't get this to run on my dev server. In any case, it results in the same issue at CF where it can't bundle the node version. Maybe distributing two separate packages??

https://github.com/simpleauthority/astro-icon/tree/feat/ssr-support

jacobsandersen avatar Aug 24 '22 11:08 jacobsandersen

hello @simpleauthority i opened an issue #52 and i suggested to use a javascript native approach and not depend on nodejs , i can help with a fix , i just need some hints or approach, i will search for alternatives later

mrabtikhalid avatar Aug 25 '22 07:08 mrabtikhalid

hello @simpleauthority i opened an issue #52 and i suggested to use a javascript native approach and not depend on nodejs , i can help with a fix , i just need some hints or approach, i will search for alternatives later

Yeah I am not sure if there is anything that is plain JS or not. I'll have to keep looking, too.

jacobsandersen avatar Aug 26 '22 03:08 jacobsandersen

I also can't deploy on netlify with edge functions. I know it's not ideal because the svg won't get optimized, but for temporary solution removing all the svgo related stuff wouldn't fix the problem? I just don't know how to do it.

pedrosc94 avatar Aug 30 '22 20:08 pedrosc94

That would work but not sure if it would be an acceptable solution for @natemoo-re. It might be as simple as commenting out this line https://github.com/natemoo-re/astro-icon/blob/main/packages/core/lib/utils.ts#L83 for now but not sure if we'd need to go and actually remove all the code for it to get it to run properly.

jacobsandersen avatar Aug 30 '22 22:08 jacobsandersen

I endup using the library without the svg compression for the moment,

mrabtikhalid avatar Sep 03 '22 17:09 mrabtikhalid

Yeah I also went for a workaround for now, and created a RootLayout.astro with <script src="https://cdnjs.cloudflare.com/ajax/libs/iconify/3.0.0-beta.3/iconify.min.js"></script> and then I just import that layout to my other layouts, then I use <span class="iconify-inline" data-icon="simple-icons:twitter" data-height="24"></span>

pedrosc94 avatar Sep 04 '22 14:09 pedrosc94

But would would prefer to use this package just for the sake of simplicity

pedrosc94 avatar Sep 04 '22 14:09 pedrosc94

Same problem deploying to deno.

cami7ord avatar Sep 13 '22 06:09 cami7ord

bumping this up!

lasfito avatar Sep 16 '22 02:09 lasfito

I have the same problem with SSR on Netlify.

skreutzberger avatar Sep 21 '22 15:09 skreutzberger

I think we should separate the svg compression , and each server library (netlify/deno....) should implement its own compression library, we should add just an interface for compression

mrabtikhalid avatar Sep 23 '22 20:09 mrabtikhalid

Is there a workaround for the time being?

jjjrmy avatar Sep 27 '22 01:09 jjjrmy

This definitely seems like something we should work to improve upon. I'm not sure what the best route is yet but did notice that there is a browser version that we could import which should allow it to work with most environments.

stramel avatar Oct 20 '22 17:10 stramel

I was looking into this and think we could utilize the browser version of svgo and then allow for specifying a custom optimizer in a file like src/icons/_optimize.ts. Does this sound like it would be useful?

stramel avatar Oct 21 '22 17:10 stramel

I tried to fork astro-icon to use the browser version before, but was unsuccessful. If you can figure out how to get that to work, then yes!

jacobsandersen avatar Oct 23 '22 12:10 jacobsandersen

Will be working to shift svgo over to build time optimization and only apply it to local icons.

stramel avatar Oct 24 '22 16:10 stramel

Figured out a simple workaround (thanks to @simpleauthority 's comment).

We need to patch the astro-icon package. More specifically we need to update line 5 of lib/utils.ts .

I'm using pnpm and it has built-in patching feature. Read this: https://pnpm.io/cli/patch (watch the video on that page)

(Note: yarn v2+ also has built-in patching feature, for npm you could use https://github.com/ds300/patch-package)

Step 1: run pnpm ls to find out the version

image

Step 2: run pnpm patch [email protected] (adjust version number)

image

Step 3: open the /tmp/... folder in VS Code or any other editor

code /tmp/24c156128bab6fdefc8998671638ad98 &

step 4: edit lib/utils.ts and Save the file

// change line 5 from this
import { optimize as optimizeSVGNative } from "svgo";

// to this
import { optimize as optimizeSVGNative } from "svgo/dist/svgo.browser";

image

Step 5: commit the patch using pnpm patch-commit command

# example (adjust the folder path)
pnpm patch-commit /tmp/24c156128bab6fdefc8998671638ad98

Done

Run the build command

image

Update

Although astro build is working with the patch, astro dev is now throwing error. So I reverted that patch for now.

rupampoddar avatar Oct 26 '22 14:10 rupampoddar

Looked into this today and it seems there will need to be some effort put into getting this to SSR properly in non-node environments. Will be working on this as a top priority.

stramel avatar Oct 26 '22 18:10 stramel

I tried the patch @rupampoddar suggested, but I am getting this error

image

image

what is the svgo version that you are using?

it looks if I remove svgo from the dependencies, it works

demiro avatar Oct 31 '22 10:10 demiro

Hi @demiro , Have you added external: ["svgo"] in astro config ?

Here's my astro.config.mjs file:

import { defineConfig } from "astro/config";
import cloudflare from "@astrojs/cloudflare";

export default defineConfig({
  output: "server",
  integrations: [
    // ...
  ],
  adapter: cloudflare({
    mode: "directory",
  }),
  vite: {
    define: {
      // ...      
    },
    ssr: {
      external: ["svgo"], // <=== THIS
    },
  },
});

Update on the patch

Currently my patched file ([email protected] - lib/utils.ts) looks like the following. Both build and dev is working. (Note: I am not using any local icon files)

/// <reference types="vite/client" />
import { SPRITESHEET_NAMESPACE } from "./constants";
import { Props, Optimize } from "./Props";
import getFromService from "./resolver";
// import { optimize as optimizeSVGNative } from "svgo";    // <==== (PATCH) comment out or remove line

// Adapted from https://github.com/developit/htmlParser
const splitAttrsTokenizer = /([a-z0-9_\:\-]*)\s*?=\s*?(['"]?)(.*?)\2\s+/gim;
const domParserTokenizer =
  /(?:<(\/?)([a-zA-Z][a-zA-Z0-9\:]*)(?:\s([^>]*?))?((?:\s*\/)?)>|(<\!\-\-)([\s\S]*?)(\-\->)|(<\!\[CDATA\[)([\s\S]*?)(\]\]>))/gm;

const splitAttrs = (str) => {
  let res = {};
  let token;
  if (str) {
    splitAttrsTokenizer.lastIndex = 0;
    str = " " + (str || "") + " ";
    while ((token = splitAttrsTokenizer.exec(str))) {
      res[token[1]] = token[3];
    }
  }
  return res;
};

// (PATCH) update this function
function optimizeSvg(
  contents: string,
  name: string,
  options: Optimize
): string {
  return contents;    // <==== return content without doing anything,
}

PNPM & Cloudflare Pages

When using pnpm, you also need to make some changes to the build configuration on cloudflare. Follow this : https://community.cloudflare.com/t/add-pnpm-to-pre-installed-cloudflare-pages-tools/288514/5

rupampoddar avatar Oct 31 '22 14:10 rupampoddar

@rupampoddar Thank you for your efforts! I can confirm that your solution works with local svg's too :)

steveninety avatar Dec 01 '22 20:12 steveninety

Uhm is this still failing for anyone else using pnpm with vercel?

FlipFloop avatar Jan 06 '23 08:01 FlipFloop

@rupampoddar thank you, so much. This is working for me with Vercel, and also for local icons

josemarcilio avatar Mar 27 '23 22:03 josemarcilio

Thanks very helpful

herbras avatar May 20 '23 18:05 herbras

Here is a way without using pnpm patch, you can configure the vite compilation option of astro.config.mjs

import { defineConfig } from "astro/config";
import cloudflare from "@astrojs/cloudflare";

// https://astro.build/config
export default defineConfig({
  output: "server",
  adapter: cloudflare(),
  vite: {
    resolve: {
      alias: {
        "svgo": import.meta.env.PROD ? "svgo/dist/svgo.browser.js" : "svgo"
      }
    }
  }
});

hongfanmeng avatar Jun 06 '23 07:06 hongfanmeng