marko-js / marko

A declarative, HTML-based language that makes building web apps fun
https://markojs.com/
MIT License
13.39k stars 645 forks source link

The "no-update" attribute doesn't work anymore #1694

Closed xtremespb closed 6 months ago

xtremespb commented 3 years ago

Marko Version: 5.10.3

Details

The "no-update" attribute as described on Marko Docs doesn't work anymore when a page is generated on a server side.

For example, there's such code in a Marko page:

<div class="z3-mf-ace" no-update/>

In case when this page is rendered on a server side and loaded in a browser, the following error is being displayed in Chrome Console:

VElement.js:160 Uncaught DOMException: Failed to execute 'createElementNS' on 'Document': The qualified name provided is empty.
    at VElement.___actualize (webpack:///../../../../node_modules/marko/src/runtime/vdom/VElement.js?:160:18)
    at insertVirtualNodeBefore (webpack:///../../../../node_modules/marko/src/runtime/vdom/morphdom/index.js?:77:26)
    at morphChildren (webpack:///../../../../node_modules/marko/src/runtime/vdom/morphdom/index.js?:391:13)
    at insertVirtualNodeBefore (webpack:///../../../../node_modules/marko/src/runtime/vdom/morphdom/index.js?:92:9)
    at morphChildren (webpack:///../../../../node_modules/marko/src/runtime/vdom/morphdom/index.js?:391:13)
    at insertVirtualNodeBefore (webpack:///../../../../node_modules/marko/src/runtime/vdom/morphdom/index.js?:92:9)
    at morphChildren (webpack:///../../../../node_modules/marko/src/runtime/vdom/morphdom/index.js?:391:13)
    at morphComponent (webpack:///../../../../node_modules/marko/src/runtime/vdom/morphdom/index.js?:130:5)
    at morphChildren (webpack:///../../../../node_modules/marko/src/runtime/vdom/morphdom/index.js?:203:13)
    at morphEl (webpack:///../../../../node_modules/marko/src/runtime/vdom/morphdom/index.js?:660:7)

That's what happening when no-update is being used. When removed, the error is gone.

Expected Behavior

The no-update attribute works as expected, without errors.

Actual Behavior

The page doesn't render correctly as the error is happening.

Possible Fix

Maybe something to change here: marko/src/runtime/vdom/VElement.js?

___actualize: function (doc, parentNamespaceURI) {
    var tagName = this.___nodeName;
    var attributes = this.___attributes;
    var namespaceURI = DEFAULT_NS[tagName] || parentNamespaceURI || NS_HTML;

    var flags = this.___flags;
    var el = doc.createElementNS(namespaceURI, tagName);

Your Environment

Steps to Reproduce

git clone https://github.com/xtremespb/zoia.git
cd zoia
npm i
npm run config
npm run build-dev
npm run setup-all
npm run cli -- --user admin --email admin@zoiajs.org

When finished, go to http://127.0.0.1:3001/admin/pages (use admin/password as username/password) and create a page. Then, click "Edit" action to edit this page, and refresh the page in Chrome. The admin page for Pages editor will be rendered wrong and you will get the error in Chrome Console as described above.

Stack Trace

See above.

DylanPiercey commented 3 years ago

@xtremespb if you get a debugger on:

var tagName = this.___nodeName;
var namespaceURI = DEFAULT_NS[tagName] || parentNamespaceURI || NS_HTML;

What do you see as namespaceURI and tagName? Is it possible for you to create a repro I could look at?

xtremespb commented 3 years ago

@DylanPiercey on the first breakpoint I got the following:

attrValue: undefined
attributes: {id: "pageEditForm_content_ace", style: null}
doc: document
el: undefined
flags: undefined
namespaceURI: "http://www.w3.org/1999/xhtml"
parentNamespaceURI: "http://www.w3.org/1999/xhtml"
tagName: "div"
this: VElement
type: undefined

On the next breakpoint (where the error is actually coming from):

attrName: undefined
attrValue: undefined
attributes: {}
doc: document
el: undefined
flags: 0
namespaceURI: "http://www.w3.org/1999/xhtml"
parentNamespaceURI: "http://www.w3.org/1999/xhtml"
tagName: ""

I will try to reproduce this bug on a minimum repo.

xtremespb commented 3 years ago

So looks like the no-update brings an empty tagName:

        <div
            id=`${input.id}_${state.item.id}_ace`
            style=(
                state.modeAce === "ace"
                    ? ""
                    : "clip:rect(0 0 0 0);width:1px;height:1px;margin:-1px;overflow:hidden"
            )>
            <div class="z3-mf-ace" no-update/> 
        </div>
xtremespb commented 3 years ago

I've built a simple app using Marko and Fastify but cannot reproduce this error, no-update works in this simple case:

<div>
    OK Computer
    <div no-update>
        Hello world
        ${state.test}
    </div>
    <div>
        ${state.test}
    </div>
    <button on-click("onButtonClick")>
        Hey
    </button>
    <div
            id=`test_ace`
            style=(
                state.modeAce === "ace"
                    ? ""
                    : "clip:rect(0 0 0 0);width:1px;height:1px;margin:-1px;overflow:hidden"
            )>
            <div class="z3-mf-ace" no-update/> 
        </div>
</div>

So the problem seems that this bug comes from nested tags. @DylanPiercey could you please take a look on my steps to reproduce from the first post?

xtremespb commented 3 years ago

Long story short, I've figured out how to fix this error and what's actually happening.

The first (and the biggest) problem is that there is no working beautify plugin for Visual Studio Code, the old one was designed for Marko 4 and doesn't work anymore. So I've investigated how to solve it and have found a @marko/prettyprint which is able to format Marko code just fine. The only problem that it add some blank lines (probably I need to report this bug, too) like this:

        <div class="z3-ap-main-footer">
            ZOIA &copy; 2019-${new Date().getFullYear()} Michael A. Matveev.&nbsp;<a
                href="https://opensource.org/licenses/MIT"
                target="_blank">
                ${out.global.i18n.t("license.mit")}
            </a>.

        </div>

    </body>

So I've written a simple script which removes all empty lines:

/* eslint-disable no-console */
const fs = require("fs-extra");
const path = require("path");
const {
    prettyPrintFile,
} = require("@marko/prettyprint");

const processMarkoFiles = async dir => {
    const files = await fs.readdir(dir);
    for (const file of files) {
        const filePath = path.resolve(`${dir}/${file}`);
        const stat = await fs.stat(filePath);
        if (stat.isDirectory()) {
            await processMarkoFiles(filePath);
        } else if (path.extname(filePath) === ".marko") {
            console.log(filePath);
            prettyPrintFile(filePath, {});
            const fileData = (await fs.readFile(filePath, "utf8")).replace(/\\\$\{/gm, "${").replace(/\\\$!\{/gm, "$!{").split(/\n|\r\n/);
            let tagFound = false;
            let fileDataNew = "";
            for (const line of fileData) {
                if (!tagFound && line.match(/^</)) {
                    tagFound = true;
                }
                if (tagFound && line.match(/^\n|\r\n$/)) {
                    // Do something, we're not adding an empty line
                } else {
                    // Add a new line, it's not empty
                    fileDataNew += `${line}\n`;
                }
            }
            fileDataNew = tagFound ? `${fileDataNew.trim()}\n` : `${fileData.join("\n")}\n`;
            await fs.writeFile(filePath, fileDataNew, "utf8");
        }
    }
};

(async () => {
    await processMarkoFiles(path.resolve(`${__dirname}/../../src`));
})(); 

It was working as a charm but has broken the no-update as I mentioned before. After some analysis I've figured out that the previous version of code was working just fine and noticed that marko/prettyprint sometimes adds blank spaces on closing tags:

image

I think that Marko interprets such blanks as "null" tags so the error described here is coming. This fix is quite trivial: trimRight() the processed lines so no blank spaces will be left on the right side.

Don't know how to fix this correctly but I think that this behavior is not correct, anyway.

DylanPiercey commented 6 months ago

I'm assuming this is resolved by https://github.com/marko-js/marko/pull/2212 (or earlier release). Going to close for now, feel free to reopen otherwise.