Add asset support to :play

[ObsidianSnoo]

💸 TL;DR

Adds support for assets, both virtual and from the local filesystem.

📜 Details

• VirtualFS:
  • Leverages IndexedDB to persist assets
• Local filesystem access:
  • Load assets from a ZIP file
  • Load directly from a folder
    • Depends on the experimental File System Access API
• Files are converted to blob: URLs and served offline

🧪 Testing Steps / Validation

✅ Checks

• CI tests (if present) are passing
• Adheres to code style for repo
• Contributor License Agreement (CLA) completed if not a Reddit employee

[niedzielski]

👍 👍 looking good! I left some feedback on the singleton and subclass approaches that I'm hoping we can find an alternative for. I like a lot of the other stuff I saw! thank you for the patch!

[niedzielski]

I understand that this is a ReactiveElement to participate in @property and dispatch but I don't understand why. here's my thinking: 1) tying a model to UI is complex and we should avoid if practical 2) if I return a new object and assign it to my @state in play-pen, it magically percolates down to all components that take a .playAssets property. I see your patch already does 2 so I think that either the issue is that a child is mutating this object directly instead of dispatching event to request mutation or the reactivity is not needed.

[ObsidianSnoo]

It doesn't need to be a ReactiveElement in particular, but this app uses lit-html and is more convenient than using the underlying HTMLElement interfaces directly. This element still participates in emitting events and passing attributes/properties around, but doesn't render anything so it doesn't need to be a LitElement.

[niedzielski]

an element can only have one parent so it breaks the data flow promise of children emitting events upward and parents pushing data down. an element can't subscribe to events without using PlayAssets as a side-channel. it's not as problematic as using globalThis but it does establish an event bus that the rest of the app doesn't use.

[ObsidianSnoo]

I think you're still considering the original approach of elements nesting <play-assets> when they wanted access. This version is only used once in <play-pen> and contains all the filesystem logic. As of the latest commit the asset state is owned by <play-pen> and passed down into <play-assets> as well as down the tree into the various elements that read from it. All fields in AssetsState are readonly and changes made by <play-assets> are emitted as an assets-updated event with a new copy for <play-pen> to receive and then apply back down the tree.

[niedzielski]

I understand that most of the events now dispatch upwards from the dialogs and there's no mutation there 👍

I don't understand why assets is a reactive element or dispatches anything. I think tying up what is largely a model with UI was one of the things that made Studio hardest to reason about. testing UI is so much harder than testing models but what really sucks is reasoning about Lit lifecycles in a model. if it's an async issue, I'd return a new copy (or current copy if no change) of the model that play-pen awaits and assigns to drive a UI change.

[ObsidianSnoo]

I'd say I'm using it for encapsulation in this case. The code that actually interacts with the filesystem needs to go somewhere and making <play-assets> an element means that I can continue using the same pattern for registering for events as everything else instead of either creating a class that extends EventTarget or rolling my own simple callback/listener loop and managing registration in connectedCallback/disconnectedCallback. Essentially, it's a cosmetic choice 😅

[niedzielski]

I think this will drive another state change. should we use willUpdate()?

[ObsidianSnoo]

Modifying state in update() does not trigger further updates/renders: https://lit.dev/docs/api/ReactiveElement/#ReactiveElement.update Though willUpdate() would be fine since it occurs before other callbacks, it's not necessary here since _accept isn't critical to rendering or other updates since it's for the file picker dialog.

[niedzielski]

I understand that most of the events now dispatch upwards from the dialogs and there's no mutation there 👍

I don't understand why assets is a reactive element or dispatches anything. I think tying up what is largely a model with UI was one of the things that made Studio hardest to reason about. testing UI is so much harder than testing models but what really sucks is reasoning about Lit lifecycles in a model. if it's an async issue, I'd return a new copy (or current copy if no change) of the model that play-pen awaits and assigns to drive a UI change.