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

[3.0] Saving the value of 'this' is broken in class constructor but works with transform-es2015-classes #313

Closed
Jessidhia opened this issue Jun 29, 2016 · 12 comments
Labels

Comments

@Jessidhia
Copy link

The code in the following gist (which happens to be based on the example at facebook/react#7094 (comment)) explains the issue in more detail:

https://gist.github.com/Kovensky/0ce87560e4b3c93cd77475fdcf94b610

Using react-hot-loader@3.0.0-beta.2. Note that the component in that gist is not inside an AppContainer, but the "real world" version of it ultimately is, and that one also has the same error behaviour.

The same problem also happens when using transform-class-properties and arrow function properties, as they're bound at construction time.

@Jessidhia Jessidhia changed the title Function.prototype.bind(this) is broken in class constructor without transform-es2015-classes Saving the value of 'this' is broken in class constructor without transform-es2015-classes Jun 29, 2016
@Jessidhia
Copy link
Author

This seems related to #242. If it's not supported at all, I wonder, then, why did it (appear to?) work when transform-es2015-classes was used...

@Jessidhia Jessidhia changed the title Saving the value of 'this' is broken in class constructor without transform-es2015-classes [3.0] Saving the value of 'this' is broken in class constructor but works with transform-es2015-classes Jul 4, 2016
@jkeljo
Copy link

jkeljo commented Jul 8, 2016

I'm seeing this as well. The React docs recommend this pattern of reassigning functions in the constructor for a couple of purposes:

But even the presence of react-hot-loader is enough to break the pattern (you don't actually need to make a code change). From playing around in the debugger, it looks like react-hot-loader is cloning the objects in a way that doesn't update the binding of already-bound member functions.

I created a repro repo here before I realized that this was the same issue.

@MarshallOfSound
Copy link

Came across this today while doing some work integrating react-hot-loader into a pure Electron dev workflow. In Electron you only need to target latest stable Chromium which means that classes are already implemented natively. This issue is much more prominent in that space because people just don't use the class transform as it isn't required.

If someone can point be in a good direction I'll be happy to dive into this at some point and get it sorted.

@wkwiatek
Copy link
Collaborator

@MarshallOfSound it's the issue of react-proxy. If you wanted to start with this, just try to do a simple proxy object (with react-proxy@next), and play with a dubugger. We'll dive into react-proxy more once we have docs ready for RHL as it's the highest prio now.

@MarshallOfSound
Copy link

@wkwiatek OK so I spent a while inside of react-proxy and like half got it working, it's insanely complex so this could take a while I feel 😆

@merges
Copy link

merges commented Apr 12, 2017

Is there a workaround for those of us who are experiencing this issue today?

@penx
Copy link

penx commented Sep 12, 2017

I had this issue when trying to switch from babel-preset-es2015 to babel-preset-env. Adding transform-es2015-classes to the included transforms resolved it for me:

presets: [
  ['env', {
    targets: {
      chrome: 60
    },
    include: ['transform-es2015-classes']
  }],
  'stage-1',
  'react'
],
plugins: ['react-hot-loader/babel'] 

@gregberge
Copy link
Collaborator

This is a known problem, we have to update babel plugin to handle native classes. Help welcome!

@can-cc
Copy link

can-cc commented Sep 20, 2017

+1
wait update

@ChuckJonas
Copy link

Also looking for a fix here!

@gregberge gregberge removed this from the v3.0 milestone Oct 15, 2017
@kyleramirez
Copy link

+1
Thanks for being on this. Waiting patiently for an update.

@gregberge
Copy link
Collaborator

Fixed in v4. Also added a modern example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests