spotlight icon indicating copy to clipboard operation
spotlight copied to clipboard

Spotlight Code is bundled twice in Astro Dev mode

Open Lms24 opened this issue 2 years ago • 9 comments

Lms24 avatar Nov 21 '23 17:11 Lms24

let's see if we can keep this closed

Lms24 avatar Nov 22 '23 13:11 Lms24

nope...

Lms24 avatar Nov 22 '23 15:11 Lms24

OK So I think I figured out the cause:

We bundle two versions of Spotlight code in different places and dispatch to the "wrong" event target

  • One time, the spotlight code is injected into the page bundle. This is fine and works just like our SDK in Astro. This version bootstraps itself, creates the shadow root, mounts the debugger window and waits for someone to open the app. For example, its eventTarget is listening for an "open" event.
  • The other time, the spotlight code is added to the Astro toolbar plugin bundle. Inside the init function of our Astro toolbar plugin file, we call Spotlight.open(). This call dispatches an "open" event to spotlight's eventTarget. However, the event target that this is dispatched on, is the one from the toolbar plugin bundle. While it should actually trigger the other target.

I think we have two solutions:

  1. Simply initalize and bootstrap Spotlight within the plugin bundle and don't inject it into the page bundle. This would be easiest but has two disadvantages:
  • we can't initialize Spotlight if the toolbar is not activated by users
  • we/users can't pass options to the Spotlight.init call in the plugin because currently, you can't pass any data/arguments to the plugin initialization function.
  1. Put the event target onto the global object. This makes sure what there's only one instance and we can listen and dispatch to it from both instances.

I think, given the limitations of 1, I'm gonna go with 2.

Oh btw I have no idea about why this didn't cause problems within the monorepo. My best guess is that Vite and pnpm resolve and bundle dependencies differently than when a package is added as a dependency. This gives me trust issues in our monorepo 😅

Lms24 avatar Nov 22 '23 18:11 Lms24

So, this has all to do with how and where we import spotlight from.

  1. In the overlay plugin, we import * as Spotlight from @spotlightjs/core because importing from @spotlightjs/astro (i.e.its own package) throws errors
  2. In the page snippet we inject, we import * as Spotlight from @spotlightjs/astro because importing from @spotlightjs/core throws an error (at least in pnpm which requres all deps to be directly specified in package.json).

Not sure if we can get around this easily...

Lms24 avatar Nov 22 '23 19:11 Lms24

I don't think double-bundling is something that should happen. I could see this as perhaps being a race-condition where both modules are loaded in parallel resulting in them being double-loaded.

What I'd try first is listing these packages in Vite's optimizedDeps config here: https://vitejs.dev/config/dep-optimization-options.html#optimizedeps-include

Doing this will cause Vite to pre-bundle them, in which case it should see that there is common dependencies between them and prevent the duplication. If that doesn't work, let me know and I'll think of something else.

How you would do that in an Astro integration is with updateConfig

matthewp avatar Nov 22 '23 21:11 matthewp

@matthewp thanks for looking into this!

I gave it a quick shot and optimizedDeps seems to fix it on first glance. However, in pnpm-based projects, vite will complain about not being able to resolve @spotlightjs/core. I think that's because the package is a dependency of @spotlightjs/astro and therefore not explicitly declared in users' package.jsons.
If I only include "@spotlightjs/astro" in the optimizedDeps array, the original problem still persists (which makes sense afaict)

Any further ideas?

Fwiw, I pushed a workaround in #121 which also resolves the problem but it doesn't fix the double bundling.

Lms24 avatar Nov 23 '23 07:11 Lms24

Gonna remove from the 1.0 milestone because it's unlikely that we fix this properly before 1.0. Also edited the title to be more precise.

Lms24 avatar Nov 28 '23 18:11 Lms24

@Lms24 how can I tell if this is still an issue or not after the general Vite plugin approach?

BYK avatar Jul 25 '24 22:07 BYK

how can I tell if this is still an issue or not after the general Vite plugin approach?

I guess we'd need to remove putting the spotlight event target (see eventTarget.ts) onto the global object and only rely on the event target in the module scope. Then try to open/close spotlight in the Astro dev toolbar.

However, given the age of this issue, if things work fine for now (with the global even target), I suggest we close this and reopen/open if we get reports. wdyt?

Lms24 avatar Jul 29 '24 09:07 Lms24

Closing as we no longer support Astro Dev Toolbar

BYK avatar Sep 26 '25 20:09 BYK

@BYK @Lms24 shouldn't this package be removed from NPM?

https://www.npmjs.com/package/@spotlightjs/astro

I had it bookmarked for a new project, and just went to install it. Install went fine, but then Oxlint warned me that @sentro/astro has no default export. Came here and found this.

JamesJosephFinn avatar Oct 13 '25 23:10 JamesJosephFinn

@BYK @Lms24 Are the spotlight docs then now inaccurate when they state:

Spotlight provides a number of convenience utilities to run the Sidecar, and in some cases it will run automatically with an integration (suchy [sic] as in Astro).

Sad to see this dev toolbar integration is no longer supported. Was looking forward to trying it out.

As I said, install went fine, but attempting to access certain functionality in sidecar emits the error to terminal:

[ERROR] [toolbar] Failed to load dev toolbar app from /node_modules/.pnpm/@[email protected]_@[email protected][email protected]_@[email protected][email protected]_beba982c4c5d5f47933ecf7fb2fd56fa/node_modules/@spotlightjs/astro/src/overlay/index.ts: Importing a module script failed.

JamesJosephFinn avatar Oct 14 '25 17:10 JamesJosephFinn