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

Get it working with SystemJS #275

Closed
gaearon opened this issue May 2, 2016 · 25 comments
Closed

Get it working with SystemJS #275

gaearon opened this issue May 2, 2016 · 25 comments

Comments

@gaearon
Copy link
Owner

gaearon commented May 2, 2016

alexisvincent/systemjs-react-hot-reloader#1

@gaearon gaearon added this to the v3.0 milestone May 2, 2016
@tyscorp
Copy link

tyscorp commented May 2, 2016

I can confirm that react-hot-loader does indeed work the same for jspm as it does webpack.

Working example repo

As far as I can see, the state is reset under the same circumstances that it resets while using webpack.

For example, inserting a new node above this line causes a state reset for both jspm and webpack, whereas changing the JSX below that line works fine for both.

@alexisvincent
Copy link

@tyscorp @gaearon I think the issue is actually with react router.

@gaearon
Copy link
Owner Author

gaearon commented May 2, 2016

Can you give beta.1 a try? It uses a different approach (#272). I haven’t had a chance to look at this yet but I might find some time in a week or two.

@alexisvincent
Copy link

@gaearon Figured out a way to reproduce.

This works

    ReactDOM.render(
      <AppContainer><App /></AppContainer>,
      rootEl
    );

this fails

    ReactDOM.render(
      createElement(AppContainer, null, 
           createElement(App),
      rootEl
    );

@alexisvincent
Copy link

@gaearon I wonder if it isn't because Im using createElement which I import from react. This should be overwritten shouldn't it?

@gaearon
Copy link
Owner Author

gaearon commented May 2, 2016

I wonder if it isn't because Im using createElement which I import from react. This should be overwritten shouldn't it?

As long as react-hot-loader/patch runs before any other imports.

@gaearon
Copy link
Owner Author

gaearon commented May 2, 2016

Might be caused by this: #276 (comment).

@alexisvincent
Copy link

@gaearon Yeah Looks about right

@gaearon
Copy link
Owner Author

gaearon commented May 2, 2016

Hmm. But no, you are using createElement, not a factory. I’ll need to look at it later but I think that in your case react-hot-loader/patch executes later than it should.

@alexisvincent
Copy link

Hmm. Ill force it to load well before to test... Also, I've never used JSX but I thought it was a source transform... How does it get access to createElement

@alexisvincent
Copy link

@gaearon I loaded react-hot-loader/patch in a promise and on completion loaded the rest. Same issue. I wonder if theres an issue with multiple versions of react?

@alexisvincent
Copy link

@gaearon Nope. Only one version of react is being loaded

@gaearon
Copy link
Owner Author

gaearon commented May 2, 2016

Yeah, hard to guess. I’ll need to look into this when I get some time.

@calesce
Copy link
Collaborator

calesce commented Oct 23, 2016

@alexisvincent Hey, I was taking a look at this and unable to install via your repo's instructions. On jspm install (with jspm beta.29), I got:
err Error looking up npm:@alexkuz/react-json-tree.
I tried again and got
err Error parsing package.json file /Users/calenewman/code/jspmloader/systemjs-react-hot-reloader/jspm_packages/npm/domain-browser@1.1.7.json

@alexisvincent
Copy link

Hi @calesce, I'm glad you replied because a few days ago I hit this issue again but had forgotten about this issue. Was going to create another. I'll get a project up for you to reproduce within the hour. Going to just see if it's the same issue.

@alexisvincent
Copy link

@calesce New repo code is up. Vastly simplified. Everything should work. Basically it seams that the issue is when using createElement instead of JSX.

@alexisvincent
Copy link

Hmm. I think i forgot AppContainer in this version. Weird that it works and doesnt even without the container. Will add it now and see if this resolves the issue

@alexisvincent
Copy link

Done, same issue

@calesce
Copy link
Collaborator

calesce commented Oct 25, 2016

OK, yeah I reproduced your issue.

Also, even with JSX, on the first edit after reloading the page, it is slow to update and unmounts the component. All edits after that are fast, and the component isn't re-mounted.

@tyscorp's project had the same issue with createElement, but it didn't have the problem with the first edit. It might be a difference between systemjs-hot-reloader and jspm-devtools (I'm not familiar with the JSPM ecosystem :D).

@alexisvincent
Copy link

@calesce yeah I still need to sort that out. It has to do with how jspm-devtools bundles files initially. Will be fixed as soon as I start pushing dependencies over http2 server push

@alexisvincent
Copy link

But glad you were able to reproduce.

@alexisvincent
Copy link

@calesce Any progress on this?

@calesce
Copy link
Collaborator

calesce commented Nov 4, 2016

@alexisvincent sorry no, not yet. When I get some extended time to look, I'll probably double check that the createElement being used is the patched version. I don't have any other leads beyond that.

@alexisvincent
Copy link

I can confirm that upon a reload, react proxy is running an update. Don't know what that means for createElement. It seems that react proxy isn't keeping the state

@wkwiatek wkwiatek removed this from the v3.0 milestone Feb 19, 2017
@gregberge
Copy link
Collaborator

This issue seems outdated, I close it, feel free to add a comment if you experienced a problem relative to jspm.

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

No branches or pull requests

6 participants