Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Illegal invocation with snippet & action #14767

Open
alex-pirogov opened this issue Dec 19, 2024 · 5 comments · May be fixed by #14783
Open

Illegal invocation with snippet & action #14767

alex-pirogov opened this issue Dec 19, 2024 · 5 comments · May be fixed by #14783

Comments

@alex-pirogov
Copy link

Describe the bug

Faced TypeError: Illegal invocation when trying to implement use:action, i don't know if it is a bug or an intended behavior

Reproduction

managed to create repl: https://svelte.dev/playground/de25c600fb804678bc8915e33166a82c?version=5.14.4

Logs

No response

System Info

System:
    OS: macOS 14.4.1
    CPU: (8) arm64 Apple M1 Pro
    Memory: 155.20 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 22.7.0 - /opt/homebrew/bin/node
    npm: 10.8.2 - /opt/homebrew/bin/npm
    pnpm: 9.14.2 - ~/Library/pnpm/pnpm
    bun: 1.1.3 - ~/.bun/bin/bun
  Browsers:
    Chrome: 131.0.6778.109
    Safari: 17.4.1
  npmPackages:
    svelte: ^5.12.0 => 5.12.0

Severity

annoyance

@paoloricciuti
Copy link
Member

That's so weird...it only happens when the code is formatted exactly like this...if you add a tab before <code it works...let me check why.

@paoloricciuti
Copy link
Member

Ok i think i roughly nailed down the problem: since inside pre the whitespace is preserved when we build the marching of the template we see that the sequence has only text nodes and we skip one child here

function flush_node(is_text, name) {
    const expression = get_node(is_text);
    let id = expression;

    if (id.type !== 'Identifier') {
        id = b.id(state.scope.generate(name));
        state.init.push(b.var(id, expression));
    }

    prev = () => id;
    skipped = 1; // the next node is `$.sibling(id)`

    return id;
}

function flush_sequence(sequence) {
    if (sequence.every((node) => node.type === 'Text')) {
        skipped += 1;
        state.template.push(sequence.map((node) => node.raw).join(''));
        return;
    }

    state.template.push(' ');

    const { has_state, has_call, value } = build_template_chunk(sequence, visit, state);

    // if this is a standalone `{expression}`, make sure we handle the case where
    // no text node was created because the expression was empty during SSR
    const is_text = sequence.length === 1;
    const id = flush_node(is_text, 'text');

    const update = b.stmt(b.call('$.set_text', id, value));

    if (has_call && !within_bound_contenteditable) {
        state.init.push(build_update(update));
    } else if (has_state && !within_bound_contenteditable) {
        state.update.push(update);
    } else {
        state.init.push(b.stmt(b.assignment('=', b.member(id, 'nodeValue'), value)));
    }
}

so the output is

var pre = root_1();
var code = $.sibling($.child(pre));
var span = $.sibling($.child(code));
var span_1 = $.child(span);

the problem is that apparent when calling child if the text node is \n the browser immediately return the element (and not the text node) so calling sibling on it returns the wrong element.

I tried to prevent the skipping if the sequence only contains one node which is a \n but it broke one test. Leaving this here in case someone more familiar with this part of the codebase want's to pick it up...i'll try to see if i can fix it later.

@paoloricciuti
Copy link
Member

Uh actually update...this is literally only happening with pre...so it must be a very weird browser behavior

@trueadm
Copy link
Contributor

trueadm commented Dec 19, 2024

We have custom logic for pre in the compiler, maybe that's causing some issue here?

@paoloricciuti
Copy link
Member

We have custom logic for pre in the compiler, maybe that's causing some issue here?

Nope it's literally the browser that does that (i also did it in codepen)...but we do need to add logic specifically for pre in this case...however when i do i get an hydration mismatch when we call reset.

@paoloricciuti paoloricciuti linked a pull request Dec 19, 2024 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants