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

Layout failing with user components #72

Open
shirakaba opened this issue Dec 27, 2021 · 3 comments
Open

Layout failing with user components #72

shirakaba opened this issue Dec 27, 2021 · 3 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@shirakaba
Copy link
Collaborator

shirakaba commented Dec 27, 2021

Root cause issue for:

After a lot of red herrings involving Node version, environment, nodegui version, qode version, shell, custom elements, etc., I think I finally understand what's going on.

v0.0.3-alpha.1 was the last release in which the starter template's layout was seen to be correct. However, in that release, {#each} and {#if} blocks didn't work correctly. So I merged the PR #60 in v0.0.4, changing how child elements were inserted, which seemed to fix that problem in all cases. Unfortunately, this PR brought an unnoticed regression in more complex UIs like that of the starter template.

So this is problematic. We don't want to roll back to the DOM algorithm used in v0.0.3-alpha.1 because it has a subtle problem that stops {#each} and {#if} blocks working. We also can't stick with the approach used since v0.0.4 (still used in the version at the time of writing,v0.1.2), because it has obvious layout issues.

Fixing {#each} and {#if} behaviour was a horribly subtle problem to solve, so I think the way forward is to stick with this current DOM model (rather than rolling back to the old DOM model) and try to fix this obvious layout problem.

@shirakaba
Copy link
Collaborator Author

shirakaba commented Dec 29, 2021

I've looked into this for a few hours. As far as I can tell, the DOM algorithm is absolutely correct (I printed out the equivalent HTML for it and it's perfect). Surprisingly, it really is user components that layout is messing up for, as can be seen if you flatten out all your user components into App.svelte.

The only clue I can find as to why user components may be breaking is that they're getting interpreted as SVG by the Svelte compiler. Our preprocessor should specifically prevent this (it sets the namespace for all .svelte files as "foreign"), so I'm really not sure why it's happening.

User components being interpreted as SVG
// WORKING: Single-component (App.svelte, with StepOne.svelte flattened into it)
// App.svelte
function create_fragment(ctx) {
	let window_1;
	let text_1;
	let t;
	let text_1_wordwrap_value;
	let window_1_minSize_value;

	return {
		c() {
			window_1 = document.createElementNS("https://svelte.dev/docs#svelte_options", "window");
			text_1 = (0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.svg_element)("text");
			t = (0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.text)("Edit App.svelte to make changes to this screen. Then come back to see your changes. The app will restart with each change, thanks to Live Reload 🔥");
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.attr)(text_1, "wordwrap", text_1_wordwrap_value = true);
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.attr)(window_1, "minSize", window_1_minSize_value = { width: 500, height: 520 });
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.attr)(window_1, "windowTitle", "Hello 👋🏽");
		},
		m(target, anchor) {
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.insert)(target, window_1, anchor);
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.append)(window_1, text_1);
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.append)(text_1, t);
			/*window_1_binding*/ ctx[1](window_1);
		},
		p: svelte_internal__WEBPACK_IMPORTED_MODULE_0__.noop,
		i: svelte_internal__WEBPACK_IMPORTED_MODULE_0__.noop,
		o: svelte_internal__WEBPACK_IMPORTED_MODULE_0__.noop,
		d(detaching) {
			if (detaching) (0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.detach)(window_1);
			/*window_1_binding*/ ctx[1](null);
		}
	};
}


// FAILING: Multi-component (App.svelte + StepOne.svelte)
// App.svelte
function create_fragment(ctx) {
	let window_1;
	let stepone;
	let window_1_minSize_value;
	let current;
	stepone = new _components_StepOne_svelte__WEBPACK_IMPORTED_MODULE_2__.default({});

	return {
		c() {
			window_1 = document.createElementNS("https://svelte.dev/docs#svelte_options", "window");
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.create_component)(stepone.$$.fragment);
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.attr)(window_1, "minSize", window_1_minSize_value = { width: 500, height: 520 });
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.attr)(window_1, "windowTitle", "Hello 👋🏽");
		},
		m(target, anchor) {
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.insert)(target, window_1, anchor);
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.mount_component)(stepone, window_1, null);
			/*window_1_binding*/ ctx[1](window_1);
			current = true;
		},
		p: svelte_internal__WEBPACK_IMPORTED_MODULE_0__.noop,
		i(local) {
			if (current) return;
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.transition_in)(stepone.$$.fragment, local);
			current = true;
		},
		o(local) {
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.transition_out)(stepone.$$.fragment, local);
			current = false;
		},
		d(detaching) {
			if (detaching) (0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.detach)(window_1);
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.destroy_component)(stepone);
			/*window_1_binding*/ ctx[1](null);
		}
	};
}

// StepOne.svelte
function create_fragment(ctx) {
	let text_1;
	let t;
	let text_1_wordWrap_value;

	return {
		c() {
			text_1 = document.createElementNS("https://svelte.dev/docs#svelte_options", "text");
			t = (0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.text)("Edit App.svelte to make changes to this screen. Then come back to see your changes. The app will restart with each change, thanks to Live Reload 🔥");
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.attr)(text_1, "wordWrap", text_1_wordWrap_value = true);
		},
		m(target, anchor) {
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.insert)(target, text_1, anchor);
			(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.append)(text_1, t);
		},
		p: svelte_internal__WEBPACK_IMPORTED_MODULE_0__.noop,
		i: svelte_internal__WEBPACK_IMPORTED_MODULE_0__.noop,
		o: svelte_internal__WEBPACK_IMPORTED_MODULE_0__.noop,
		d(detaching) {
			if (detaching) (0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.detach)(text_1);
		}
	};
}

class StepOne extends svelte_internal__WEBPACK_IMPORTED_MODULE_0__.SvelteComponent {
	constructor(options) {
		super();
		(0,svelte_internal__WEBPACK_IMPORTED_MODULE_0__.init)(this, options, null, create_fragment, svelte_internal__WEBPACK_IMPORTED_MODULE_0__.safe_not_equal, {});
	}
}

I've tried updating svelte and svelte-loader to use their new built-in support for foreign namespaces, but now for some reason the GUI is failing to appear after adding the final child component in the hierarchy.

Above all, I have no explanation whatsoever as to why my PR since the last working version could lead to this behaviour. I'm really out of ideas.

@shirakaba shirakaba added the help wanted Extra attention is needed label Dec 29, 2021
@ceremcem
Copy link

ceremcem commented Dec 30, 2021

How does svelte-nodegui differs from nodegui? What does the svelte- part do under the hood?

AFAIU, nodegui only offers a Javascript API, so any UI design system on top of it (svelte-, react-, vue- and possibly the support for Qt Designer) will have to generate the corresponding Javascript code under the hood, right?

If this is right, svelte- part of svelte-nodegui to SvelteJS is C compiler for x86_64 to C compiler for Arm. One transpiles Svelte code to Javascript code that alters the DOM directly by using use HTML DOM API and the other (this one) transpiles the Svelte code to Javascript code that alters the GUI by using nodegui's API. If this is also right, Svelte transpiler part of SvelteJS and svelte-nodegui have very distinct codebase. If this is also right, then we may take advantage of existing SvelteJS transpiler by implementing a HTML DOM API to NodeGUI API transpiler. How does that sound?

@shirakaba
Copy link
Collaborator Author

shirakaba commented Dec 30, 2021

How does svelte-nodegui differs from nodegui? What does the svelte- part do under the hood?

@ceremcem NodeGUI consists of:

  • a library of node.js->native bindings allowing you to control Qt5 from Node.js
  • a minor fork of Node.js, Qode, that syncs up its event loop with that of Qt5
  • a flexbox layout system based on yoga, allowing you to use flexbox layout on Qt5 widgets

So NodeGUI alone does not implement DOM.

React NodeGUI implements all the parts of DOM required for React support, but Svelte – given that it has no dedicated API for custom renderers – requires a few parts of DOM, and a few parts of HTML (like window, document, head, and some bits relating to styling and events). So there are some gaps to fill.

Svelte NodeGUI takes this approach:

  1. it reuses React NodeGUI's DOM interface;
  2. it fills in the remaining DOM and HTML APIs that Svelte requires;
  3. it provides typings to confer Svelte TypeScript support.

It's a chimaera of Svelte Native and NativeScript Vue. I'll admit that the way that it implements is somewhat messy as the code layout could certainly be cleaner.

image

If this is also right, then we may take advantage of existing SvelteJS transpiler by implementing a HTML DOM API to NodeGUI API transpiler. How does that sound?

So we're basically doing that already, although coupling it to Svelte. Ideally we'd get together and create a dom-nodegui library that all the renderers can use. But that's a large project to coordinate on and, for my part, my time for open source activities is very limited!

However, this is off-topic. We have a DOM implementation already. Even if we did create a dom-nodegui library (e.g. by pulling out the DOM implementation from Svelte NodeGUI into a separate project), we'd still face the problem that Svelte is for some reason failing to lay out user components correctly. This is unfortunately a Svelte internals problem that few people will be able to answer.

The path of least resistance at this point might be to go back to the DOM implementation in v0.0.3-alpha.1... but then I really worry about how to get {#each} and {#if} blocks to work (which is obviously something we can't leave broken) without literally rewriting the same PR I wrote that ended up breaking user components :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants