Skip to content

partial-app-loading example throws in updateMatchComponents when clicking around #117

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

Closed
sophiebits opened this issue Jul 25, 2014 · 22 comments

Comments

@sophiebits
Copy link
Contributor

We don't find the refs where we expect them due to the way that the PreInbox and PreDashboard components work (PreInbox's owner is Dashboard instead of PreDashboard, the handler specified by the route).

@ryanflorence
Copy link
Member

:(

That's new. Do you know if its because of the react upgrade or a change in our code?

@sophiebits
Copy link
Contributor Author

Guessing it was introduced in the change to make activeRoute a function (73570ed).

@zgotsch
Copy link

zgotsch commented Jul 26, 2014

Seems to have been introduced in 0177cdd, which is where the updateMatchComponents logic is introduced.

@ryanflorence ryanflorence added this to the v1.0 milestone Jul 27, 2014
@ryanflorence
Copy link
Member

    function updateMatchComponents(matches, refs) {
      var i = 0, component;
      while (component = refs[REF_NAME]) {
        matches[i++].component = component;
        refs = component.refs;
      }
    }

The while loop runs one more time then there are matches.

@eldh
Copy link

eldh commented Aug 20, 2014

Any progress on this or pointers on how to fix/get around it?

@eldh
Copy link

eldh commented Aug 20, 2014

A fix that seems to work for me, probably there are nicer, better, cleaner ways to get around it:

while ((component = refs[REF_NAME]) && (matches[i+1] !== undefined)) {
    matches[i++].component = component;
    refs = component.refs;
  }

@ryanflorence
Copy link
Member

^ @mjackson any issues with this code you can think of?

@mjackson
Copy link
Member

I haven't had time to track down this bug yet, but there's obviously something wrong with matches in that loop. Anyone care to make a unit test?

@alexkirsz
Copy link

This happens when a handler component creates its own __activeRoute__ ref, usually when passing down its unaltered props to a child component.

@kyle-beattie
Copy link

Any more progress or ways to solve this?

@ryanflorence
Copy link
Member

as soon as we finish server-side rendering this is number one priority before v1 release.

@ryanflorence
Copy link
Member

(but no, we haven't worked on it)

@mjackson
Copy link
Member

mjackson commented Oct 9, 2014

I'm not 100% sure (because the partial-app-loading example is currently broken) but this commit may have helped resolve this bug.

@ryanflorence
Copy link
Member

we'll need a webpack build for it

@mjackson
Copy link
Member

If you wanna use webpack, I'm down. My only issues with webpack are that the docs are a wall of text which makes it difficult for me to understand how to use it. But if we're going to use it on one example, let's just use it for everything. I don't like the idea of building some stuff with Browserify and other stuff with webpack. We should always work with Browserify tho.

@gaearon
Copy link
Contributor

gaearon commented Oct 10, 2014

Would've been nice if there was no build step for examples. Just launching webpack-dev-server and that's it. Webpack can sometimes be not documented very well, but @sokra is extremely helpful and quick to respond to issues so I don't think it's a problem.

@mjackson
Copy link
Member

@gaearon agree. no build step would be really nice.

@ryanflorence
Copy link
Member

I want webpack back now that partial app loading is fixed :)

@gaearon
Copy link
Contributor

gaearon commented Oct 10, 2014

BTW I have an exception in that loop, wonder why:

Uncaught TypeError: Cannot read property 'refs' of undefined

in refs = match.component.refs;

(But that's with my own partial loading, which worked in 0.9.3 but broke in master)

@sophiebits
Copy link
Contributor Author

I have an okay understanding of webpack configuration and can probably help if you have questions.

@ryanflorence
Copy link
Member

I'll switch it back to webpack, we still need browserify for the global build (unless webpack can do this too?)

@ryanflorence
Copy link
Member

@gaearon it is done :)

scripts/dev-examples

Then just open http://localhost:8080.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants