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

Bug in 2.2.0 – "default" element created #21

Closed
benadamstyles opened this issue Mar 15, 2016 · 14 comments
Closed

Bug in 2.2.0 – "default" element created #21

benadamstyles opened this issue Mar 15, 2016 · 14 comments

Comments

@benadamstyles
Copy link

Hi there, first of all crel is awesome and been using it for ages without any problems.

I just updated to crel 2.2.0 in a project using Babel 6 with es2015 and stage-3 presets, and am getting the following weird behaviour:

crel('div')
// --> <default>div</default>

crel('div', {'class': 'abc'})
// --> <default>​"div""[object Object]"</default>​

I don't know how to begin debugging this because I forked crel and ran npm test and all tests passed... I guess this must have to do with the new Proxy API support because that's the only major thing that's changed since 2.1.8, which works fine. Any pointers? Thanks!

@KoryNunn
Copy link
Owner

That's extremely odd. Would you be able to provide a minimal example of how this is being used (inc babel usage etc)

I get the feeling this will be some incompatibility with babel, maybe it doesn't implement Proxies correctly?

I'm installing chrome canary now to have a look, but it works fine in electron.

@benadamstyles
Copy link
Author

Yeah I was trying to work out where that default was coming from and I thought perhaps it was something to do with how babel resolves commonjs exports to es6 default exports, sounds bizarre but that's all I can think. I will try to set up a reproduction.

@benadamstyles
Copy link
Author

Hmm, works fine here which suggests it is to do with module imports. I'll try setting up a repo with my environment but it might be difficult! Any ideas how I could debug this in my own project?

@KoryNunn
Copy link
Owner

This fiddle also works fine when babel is turned off, in canary (51.0.2679.0): https://jsfiddle.net/q1dkv5mp/ (it wont work in current normal chrome 48.0.2564.116)

So I'm not really sure how to help without more info on how you're using it.

@benadamstyles
Copy link
Author

Ok I'll keep digging, thanks for trying.

@KoryNunn
Copy link
Owner

oh i know whats happening.

under the covers somewhere, babel/something is doing:

if(module.default){
    return module.default;
}

And because crel returns a bound fn with the string default as the first param, the rest of the args are getting shifted.

The exact same output would be achieved via:

var borked = crel.bind(null, 'default');

borked('div', {class: 'things'}, 'etc');

@benadamstyles
Copy link
Author

Ah ok! So is this something that should be handled by crel, or is it a bug in babel / webpack / something else?

@KoryNunn
Copy link
Owner

This is absolutely a bug in whatever is doing the default check. I'm checking babel at the moment, and I'll have a look in webpack after that, but it'll be a pretty big haystack..

@KoryNunn
Copy link
Owner

Actually if you have this breaking, add this:

if(key === 'default'){
throw new Error('boom');
}

after this line: https://github.com/KoryNunn/crel/blob/master/crel.js#L148

in your node_modules version of crel, and past the stack here.

@benadamstyles
Copy link
Author

Ok so webpack+babel is outputting the following:

overlay = (0, _crel2.default)('div', { 'class': 'modal-overlay' }, el)

This ends up using the Proxy API and trying to create a 'default' element.

@KoryNunn
Copy link
Owner

I can probably fix this by adding special handling around the default prop, but I don't like that as a solution.

The previous version is identical other than the Proxy support, so I'd go with it until the issue is resolved in babel/webpack

@benadamstyles
Copy link
Author

I agree that it's not an ideal solution. But might it be a bit dangerous to automatically call crel() on any property of crel? If someone wanted to extend crel, or monkey-patch it, they would get the same error that's difficult to debug. Maybe there should an option to opt out of Proxy support altogether?

@KoryNunn
Copy link
Owner

Actually I'll throw up a PR in a sec that solves all proxy issues

@KoryNunn
Copy link
Owner

v3 is published.

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