sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
79.65k stars 4.22k forks source link

Svelte 5: async function attached to an element inside `each` call breaks updates and fails to `reconcile` [Adobe UXP] #13956

Open kapooostin opened 4 days ago

kapooostin commented 4 days ago

Describe the bug

I have several sets of actions designed as tabs. Inside one of tabs there is a set of elements with onclick actions that call an inner async Photopshop API function. The set is rendered with {#each} block. After the click the action is performed OK, the result is returned, but tab switching breaks. From that moment on the call to each in reconsile cannot find get_key function.

Reproduction

It's rather an edge case, because I could only replicate it in Adobe UXP environment and only when an async action in question is a call to Adobe Photopshop host API, and not a regular JS async function. It all worked in Svelte 4.

Here is a playground with the basic setup of the script. A simple delayed Promise in InnerTabs.svelte:make works fine both in the browsers and in my Photoshop plugin. But if I add there a call to a Photoshop API function, and click a button, it fails to update UI from that moment onwards.

In compiled code it's this:

function reconcile(array, state, anchor, render_fn, flags, get_key) {
  ...
  for (i = 0; i < length; i += 1) {
    value = array[i];
    key = get_key(value, i); // <-- ERROR ORIGINATES HERE
    item = items.get(key);
  ...
  }
}

Getting up the stack get_key refers to the index function in each:

/**
 * @param {any} _
 * @param {number} i
 */
function index(_, i) {
  return i;
}

As it simply returns its second argument, when I replace it with

key = i;

in compiled code, the plugin works without a hitch.

Idk, why it might behave like that. I have queueMicrotask shimmed with this package. Just in case it might matter somehow.

Logs

​ Uncaught TypeError: get_key is not a function
    at reconcile (VM136 index-DMrD_uma.js:2612)
    at VM136 index-DMrD_uma.js:2541
    at update_reaction (VM136 index-DMrD_uma.js:1154)
    at update_effect (VM136 index-DMrD_uma.js:1275)
    at create_effect (VM136 index-DMrD_uma.js:594)
    at block (VM136 index-DMrD_uma.js:692)
    at each (VM136 index-DMrD_uma.js:2522)
    at InnerTabs (VM136 index-DMrD_uma.js:3832)
    at RenameLayer (VM136 index-DMrD_uma.js:3894)
    at VM136 index-DMrD_uma.js:2883

System Info

System:
    OS: macOS 14.6.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 1.91 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 22.1.0 - ~/.volta/tools/image/node/22.1.0/bin/node
    Yarn: 1.22.21 - ~/.volta/tools/image/yarn/1.22.21/bin/yarn
    npm: 10.8.3 - ~/.volta/tools/image/npm/10.8.3/bin/npm
    bun: 1.1.31 - ~/.volta/bin/bun
  Browsers:
    Chrome: 130.0.6723.70
    Safari: 17.6
  npmPackages:
    svelte: ^5.0.5 => 5.0.5
  Adobe Photoshop: 25.12.0
    UXP: 7.4

Severity

blocking an upgrade

kapooostin commented 2 days ago

I nailed the problem. It's on the Adobe side mostly, but I have a point to discuss on the Svelte side also.

After executing a Photoshop API call the top level function index (from internal/client/dom/blocks/each) is overwritten and becomes an integer because of a silly mistake in their codebase:

for (index = 0; index < varArgCount; ++index) { // let is missing
  jsArgs.push(theArgs[index + fixedArgCount]);
}

I'll file the issue with them, though they are famous for being slow with the fixes (their API has been returning RGB color as { red, grain, blue } for years). Meanwhile, the simplest solution seems to just globally declare index variable at the top of the script and then the function in question is exported as index$1 and never gets overwritten.

As far as I understand, index is exported from the each module to keep implementation details within the module. Would it be possible to give it less generic name? Like get_index, for example? We'd need to update the function name in the block, the exported name, and the call to it from EachBlock visitor in the transformation phase of the compiler. I could prepare a pull request if this change would be acceptable.

kapooostin commented 1 day ago

https://forums.creativeclouddeveloper.com/t/photoshop-uxp-leaks-a-global-variable-index-overwriting-any-existing-value-with-an-integer/8594/2