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

Does not work with mixins #5

Closed
oravecz opened this issue Aug 15, 2017 · 8 comments
Closed

Does not work with mixins #5

oravecz opened this issue Aug 15, 2017 · 8 comments

Comments

@oravecz
Copy link

oravecz commented Aug 15, 2017

A very common pattern emerging when developing web components is the use of mixins. Whether using functional mixins or class based mixins, one does not typically code this way:

class MyElement extends HTMLElement

Instead, the mixin looks something like this:

import Composable from './composable'
class MyElement extends Composable(HTMLElement).compose( Mixin1, Mixin 2)

<composable.js>
export default ( Base ) => {

    class Composable extends Base { ... }

    return Composable
}

or

var MyElement = Mixin1(Mixin2(HTMLElement))

I'm not sure if the plugin can be written to work with these runtime semantics.

I have tried to create a subclass of HTMLElement called FixedHTMLElement which is passed in to Composable.

class FixedHTMLElement extends HTMLElement {}

class MyElement extends Composable(FixedHTMLElement).compose( Mixin1, Mixin 2)

But this didn't work. Are there any approaches that may work with your plugin for these cases?

@WebReflection
Copy link
Owner

WebReflection commented Aug 15, 2017

wait a second ... your Composable breaks inheritance and you think it should be solved in here?

@oravecz
Copy link
Author

oravecz commented Aug 15, 2017

Breaks how? Composable extends HTMLElement

I understand how your plugin can't pick up on this syntax. I am asking if there is a workaround.

@WebReflection
Copy link
Owner

OK, let me try again:

  1. I've no idea what's your Composable doing
  2. this plugin works with native classes
  3. you're using non native 3rd parts software that doesn't work anyway with transpiled code

What do you think I should do in here? I think the problem is in your utility that doesn't work well with transpiled code.

If you want me to even think about a solution, you have to give me your Composable source code because ...

one does not typically code this way ... ( show a class extending native class )

That's all I've ever done with classes and Custom Elements so ... yes, somebody does that, dunno what you do 'cause standard ES2015 inheritance, since we're talking about preset2015 compatibility, it's all that matters to me in here.

@oravecz
Copy link
Author

oravecz commented Aug 15, 2017

Sorry, I thought that would be enough to demonstrate the point. Perhaps I was being too black/white with my intro. I should say that many developers of web components are discovering the power of mixins over the past year with libraries such as mixwith.js and articles from Component.Kitchen.

Here is the source for the ComposableMixin.

@WebReflection
Copy link
Owner

Can I have a whole page that shows there are issues once transpiled correctly with this plugin?

I need to be sure the right polyfill is used and the generated code is the expected one, or I won't be able to help.

@oravecz
Copy link
Author

oravecz commented Aug 25, 2017

Sorry I haven't come back to this for a while. My problem with extending native classes was resolved by using https://github.com/webcomponents/webcomponentsjs/blob/master/custom-elements-es5-adapter.js.

Can you explain the difference between using this "adapter" versus the approach taken by your babel plugin?

@WebReflection
Copy link
Owner

you never mentioned Custom Elements native extends.

You could've used document-register-element that works already.

the wc adapter is a workaround/patch, not needed usually on my stacks

@trusktr
Copy link

trusktr commented Oct 6, 2017

@oravecz Yep, if you read about the caveat for document-register-element in it's README, then you don't need this transform to get Custom Elements v1 working. I think that that approach is better, if you can live with those caveats.

Can you explain the difference between using this "adapter" versus the approach taken by your babel plugin?

That adapter only works in Edge 12 and up. Good luck making a bundle that works in IE and Edge. If you don't need to support IE 11 or lower, then you'll be fine.

One way to do it (make a bundle that works in IE 11 and lower as well as Edge 12 and up) is to open up that adapter file (fork it) and wrap the class extends NativeHTMLElement definition in an eval() (otherwise IE 11 and lower throw a SyntaxError during parsing). You'll also have to convert arrow functions to normal functions, and for anything lower than IE 11 you need to replace Map with a polyfill.

This will make it works everywhere (IE and lower, Edge, and the others), but another problem is that some HTTP headers block eval from being used in any code.

If your website doesn't use such headers (CSP headers), then you'll have no problem.

However, if you're making something (f.e. a library as opposed to an app) that is designed to be used by others in who-knows-which websites, then they may face broken code if they have those headers enabled.

So far, using document-register-element is easier (if you want to make it work in older browsers) because it requires no special build handling for different browsers and doesn't require your users to have to think about "if this browser load this, if that browser load that". With document-register-element, you can make a single bundle that will simply work in all browsers (as long as you deal with the caveats) and your users don't have to worry about conditionally loading things for different browsers.

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

3 participants