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

Deprecate v0 custom element reactions #13

Closed
snuggs opened this issue May 29, 2017 · 12 comments
Closed

Deprecate v0 custom element reactions #13

snuggs opened this issue May 29, 2017 · 12 comments

Comments

@snuggs
Copy link
Contributor

snuggs commented May 29, 2017

bfcb464

If we are going to migrate to v1 registration I feel we can deprecate the hooks for the v0 callbacks (i.e. attached / detached) . Will make slim a touch slimmer. ;-) <3

Let me know if you need me to PR this.

@eavichay
Copy link
Member

eavichay commented May 29, 2017 via email

@snuggs
Copy link
Contributor Author

snuggs commented May 29, 2017

Not certain you understand what I mean @eavichay.

For example: v0 callback
https://github.com/eavichay/slim.js/blob/master/src/Slim.js#L737
Can be inlined to v1 callback
https://github.com/eavichay/slim.js/blob/master/src/Slim.js#L721-L723

@eavichay
Copy link
Member

Currently it should support V1 with fallback to V0 if someone decides to use document.registerElement. If you think it should be dropped completely - your'e welcome but It smells like a breaking change so as long as it's not an issue, I'd prefer keeping it, at least until version 3 it out. Currently it looks like version 3 has real performance issues and good chances i'll drop it completely, though the code is half-size... WDYT?

@snuggs
Copy link
Contributor Author

snuggs commented May 29, 2017

🤔 great question. However I feel the current API is great and should encourage NOT having the author define the custom element themselves. I feel this is one of the main reasons to even use slim.js.Let someone else worry about the movement of the spec. Like i said the thing i learned best from ruby is enforced conventions often remove tons of edge cases of configuration. #FreeJewelry 💍 💎 .

Let's think on it a bit and make a decision later today/tomorrow. To your point we don't want to break anything. On the other side of the coin it won't be missed if no one is encouraged to use the feature. A win win win for your future self IMHO @eavichay

@snuggs
Copy link
Contributor Author

snuggs commented May 29, 2017

As you said yesterday. Can remove an extra function call on the call stack... :-) @eavichay

@eavichay
Copy link
Member

eavichay commented May 29, 2017

What if someone is not using onAdded/onRemoved but is overriding attachedCallback and calling super? It will not execute at all. It's a breaking change. I am all for this change, but I think it could wait for version 3, once the engine is rewritten.

@snuggs
Copy link
Contributor Author

snuggs commented May 29, 2017

Well @eavichay as you mentioned in the comments it's recommended to not override. Just move that comment to the README et voila (as they say in Paris 🇫🇷 )

https://github.com/eavichay/slim.js/blob/master/src/Slim.js#L734

@eavichay
Copy link
Member

viola

@eavichay
Copy link
Member

join the gitter channel please

@snuggs
Copy link
Contributor Author

snuggs commented May 29, 2017

As an addendum the one thing we lose from moving to customElementRegistry we begin to lose the ability to extend from derivatives of HTMLElement (i.e. HTMLParagraphElement) however. Which is another can of worms for later. You don't expose that api so that's not such a concern for slim.js. /cc @Robertchristopher

@eavichay
Copy link
Member

eavichay commented Jun 7, 2017

I agree. It should be deprecated.

@snuggs
Copy link
Contributor Author

snuggs commented Jun 9, 2017

Ok will work on this this weekend @eavichay

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

No branches or pull requests

2 participants