[github] Use attributes as another way to provide metadata

[DmitrySharabin]

Let's try to fix #665 (or at least begin iterating over it that I find beneficial). 😀

[LeaVerou]

Hi Dmitry, thanks for working on this!
This largely works (see my one comment though).
The reason I didn't want to go that route is that it would be a better architecture if parseURL() just parses the URL like the function name implies, returns its components, and we override them as needed in the constructor. However, a big problem with that approach is that certain components depend on others, e.g. apiCall, path.
I'm not sure if the implementation simplicity is worth the architectural tradeoff.

[DmitrySharabin]

Hi Lea!

Thank you for the comments. I agree with your thoughts and share them.

I'm not sure if the implementation simplicity is worth the architectural tradeoff.

Of course, it doesn't. Well, I continue working then. :)

[LeaVerou]

Btw, I did mean I wasn't sure. If anything, I was leaning towards that the tradeoff might be worth it. But if you want to look into it a bit more first, that's fine!

[DmitrySharabin]

Yes, I'd like to look into it a bit more. Every single time I learn a lot from these iterations. And I appreciate that. Thank you.

[DmitrySharabin]

Hi @LeaVerou,

Will you please give this PR another look when you have time? I made some adjustments and would love to have feedback from you.

Thanks!

[LeaVerou]

Thank you for working on this Dmitry. I will review properly in a bit.
Thought: I wonder if it would simplify things to express computed properties like apiCall as a writable accessor, i.e. something that returns a computed value if it doesn't have its own value, or works like a normal property if it has been set.
What do I mean by that, since you can't both specify accessors and writable: true? Something like this:

Object.defineProperty(ret, "apiCall", {
	get () { 
		return /* expression */
	}, 
	set (v) {
		delete this.apiCall;
		this.apiCall = v;
	},
	configurable: true, 
	enumerable: true
});

[DmitrySharabin]

I saw a similar code in the Mavo code base, but, honestly, I’ve never used it myself. It’s not in my blood. Yet. :) Thank you for your suggestion (question). I’ll examine this object method on MDN (or elsewhere) and give it a try. I will be back soon. ;)

[DmitrySharabin]

@LeaVerou Thank you so much for your suggestion and the brilliant following blog post that helped me A LOT. If I got everything correctly, the code we have now is more maintainable, DRY, and without any doubt, more professional. 🤞🏻 The last word is up to you, of course. ☺️

Your help is priceless. I appreciate that. Thank you! 🙇

[LeaVerou]

LGTM. Nice work, Dmitry.

I think we should move the writable getters to a helper in util.js (or even one in Bliss), but we can do that via a separate PR later.

Next step would be to add some tests for this, both for specifying params via attributes that aren't present in the URL, as well as overriding params that are present.

[DmitrySharabin]

Thank you, Lea!

Next step would be to add some tests for this, both for specifying params via attributes that aren't present in the URL, as well as overriding params that are present.

Yes, it's on my radar.

I think we should move the writable getters to a helper in util.js (or even one in Bliss), but we can do that via a separate PR later.

Vote for Bliss. :) Would love to contribute to that awesome project too. ☺️

[LeaVerou]

Vote for Bliss. :) Would love to contribute to that awesome project too. ☺️

Then hold off for a bit longer, I'm rewriting bliss to use ESM so until that ships, no new features.

[DmitrySharabin]

Deal! 🤝