stackblitz / tutorialkit

TutorialKit by StackBlitz - Create interactive tutorials powered by the WebContainer API
https://tutorialkit.dev
MIT License
232 stars 22 forks source link

feat(runtime): add `preview.pathname` #233

Closed AriPerkkio closed 1 month ago

AriPerkkio commented 1 month ago

Adds support for defining pathname for previews. Example from Vite Plugin Tutorial where I'd like to show two previews for same server:

---
type: lesson
title: Extra - Source map support
focus: /vite.config.ts
previews:
  - { port: 5173, title: "Vite" }
  - { port: 5173, title: "Inspect", pathname: "/__inspect/" }
---

This will also be useful for remult tutorial: https://learn.remult.dev/1-basics/5-live-query/2-realtime-updates - mentioned here https://github.com/stackblitz/tutorialkit/discussions/226

stackblitz[bot] commented 1 month ago

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

AriPerkkio commented 1 month ago

I would prefer if we had _availablePreviews set to Map<number, PortInfo>

export class PreviewInfo {
+  readonly portInfo: PortInfo; // initialized in constructor

I think we need to either make it Map<number, PortInfo[]> or public portInfo, so that we can mark multiple PortInfo's ready at once.

If it was public field that allows overwriting, we would do it here:

    const previewInfos = previews.map((preview) => {
      const info = new PreviewInfo(preview);
      const portInfo = this._availablePreviews.get(info.port);

      if (!portInfo) {
        this._availablePreviews.set(info.port, info.portInfo);
      } else {
        info.portInfo = portInfo; // <-- All previews sharing same port should point to same portInfo
      }

      return info;
    });

Or if we would track multiple PortInfo[] in the map, we could then mark all of them ready on webcontainer's port event.

Nemikolh commented 1 month ago

I think we need to either make it Map<number, PortInfo[]> or public portInfo, so that we can mark multiple PortInfo's ready at once.

I don't think I'm following that reasoning. A single instance of PreviewInfo uses a single port.

In your code snippet:

   const previewInfos = previews.map((preview) => {
     const info = new PreviewInfo(preview);
     const portInfo = this._availablePreviews.get(info.port);

     if (!portInfo) {
       this._availablePreviews.set(info.port, info.portInfo);
     } else {
       info.portInfo = portInfo; // <-- All previews sharing same port should point to same portInfo
     }

     return info;
   });

The logic should be changed to:

    const previewInfos = previews.map((preview) => {
      const port = PreviewInfo.parsePort(preview);
      let portInfo = this._availablePreviews.get(port);

      if (!portInfo) {
        portInfo = new PortInfo(port);
        this._availablePreviews.set(port, portInfo);
      }

      return new PreviewInfo(preview, portInfo);
    });
AriPerkkio commented 1 month ago

The logic should be changed to:

This looks good, I've applied these changes!

This allows us to have multiple PreviewInfo's referencing a single PortInfo, so that on webcontainer's port event we can mark multiple PreviewInfo's ready at once. This is what https://github.com/stackblitz/tutorialkit/pull/233#issuecomment-2283265798 was essentially about. 👍