astro icon indicating copy to clipboard operation
astro copied to clipboard

Client directives pulling in `runtime/server/index.ts` on build

Open delucis opened this issue 1 year ago • 4 comments

What version of astro are you using?

1.0.0-rc.7

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

n/a

What package manager are you using?

pnpm

What operating system are you using?

macOS / Windows / Linux

Describe the Bug

@Yan-Thomas and @hippotastic have been putting together a PR to upgrade docs to the latest RC https://github.com/withastro/docs/pull/1127/ and it has gone very smoothly but we hit one blocker:

  • Our Algolia docsearch component breaks in builds in Safari.

Hippo did some fantastic work last night debugging this and tracked the issue down to a lookbehind pattern in a regex that is not supported:

https://github.com/withastro/astro/blob/6086562a934ac31a186d7e72b5c4137116cc384f/packages/astro/src/runtime/server/index.ts#L657

The weird part is that runtime/server/index.ts should presumably not even be ending up in our bundle? The bug only manifests after a build — everything works fine in dev mode.

My vague guess is that this could be some overeager bundling with preact/compat enabled because that component is a React, which we’re rendering with Preact, but can’t be 100% sure.

Link to Minimal Reproducible Example

https://github.com/withastro/docs/pull/1127/

Participation

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

delucis avatar Aug 08 '22 09:08 delucis

Looking through the built .js bundles, I think runtime/server/index.js it's being pulled in by jsx/babel.ts as part of the client:only build support 🤔

https://github.com/withastro/astro/blob/6086562a934ac31a186d7e72b5c4137116cc384f/packages/astro/src/jsx/babel.ts#L93

tony-sull avatar Aug 08 '22 11:08 tony-sull

Ahh crap, I bet you're right @tony-sull. We should inline that.

natemoo-re avatar Aug 08 '22 15:08 natemoo-re

Reopening as @hippotastic is reporting this as unresolved. He's going to post his reproduction below to help!

delucis avatar Aug 09 '22 09:08 delucis

Thank you, @delucis! :) This is still not fixed in 1.0.0-rc.8 unfortunately.

Here's a link to my minimal reproduction of the issue: https://github.com/hippotastic/astro-component-directive-issue

You just need to clone it and run pnpm i and pnpm build to see it. I added an extra step to the build script so that it runs eslint right afterwards on the dist folder, and I added an eslint rule that detects the unsupported regular expression in the build output.

You'll see the following error then:

/Users/hippo/Dev/astro-component-directive-issue/dist/Search.61c504a0.js
  30:881  error  ES2018 RegExp lookbehind assertions are forbidden  es/no-regexp-lookbehind-assertions

✖ 1 problem (1 error, 0 warnings)

As you can see in the code of the reproduction repo, creating the bug only needs this chain:

  1. index.astro imports another Astro file Search.astro, but incorrectly uses the directive on it like so <Search client:idle />
  2. Search.astro imports a React/Preact file DocSearch.jsx, and uses it with a second client directive like so: <DocSearch client:idle />

hippotastic avatar Aug 09 '22 09:08 hippotastic

Closed by c0992e1

FredKSchott avatar Sep 07 '22 16:09 FredKSchott