emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.48k stars 4.21k forks source link

Error using `this.replaceWith` with glob route encountered upgrading from Ember 2.0.0 to 2.3.1 #12945

Open kategengler opened 8 years ago

kategengler commented 8 years ago

When upgrading EmberObserver.com to Ember 2.3.1 from Ember 2.0.0, we encountered a couple of errors around handling when an model is not found via the error action on routes to go to a 404 page:

One of the relevant lines is here: https://github.com/emberobserver/client/commit/b85d31d0c4183a060fdc20e20c986aa929f88d5e#diff-42428fe7c69c15bef5f37cff1bd734cdR15

When it's

  this.replaceWith('/not-found');

It appropriately goes to the desired route but errors with:

TypeError: this.transitionTo.apply(...).method is not a function
    at _emberRuntimeSystemObject.default.extend.replaceWith (ember.debug.js:27530)
    at _emberRuntimeSystemObject.default.extend.replaceWith (ember.debug.js:26163)
    at error (show.js:19)
    at Object.triggerEvent (ember.debug.js:28100)
    at Object.trigger (ember.debug.js:52625)
    at Object.Transition.trigger (ember.debug.js:52439)
    at ember.debug.js:52259
    at tryCatch (ember.debug.js:52958)
    at invokeCallback (ember.debug.js:52973)
    at publish (ember.debug.js:52941)
    at publishRejection (ember.debug.js:52876)
    at ember.debug.js:31521
    at Queue.invoke (ember.debug.js:327)
    at Object.Queue.flush (ember.debug.js:391)
    at Object.DeferredActionQueues.flush (ember.debug.js:192)
    at Object.Backburner.end (ember.debug.js:570)
    at Object.Backburner.run (ember.debug.js:692)
    at Object.Backburner.join (ember.debug.js:712)
    at Function.run.join (ember.debug.js:20709)
    at hash.success (rest.js:745)
    at fire (jquery.js:3099)
    at Object.self.fireWith [as resolveWith] (jquery.js:3211)
    at done (jquery.js:8264)
    at jquery.js:8605
    at Object.<anonymous> (fake_xml_http_request.js:137)
    at Object.dispatchEvent (fake_xml_http_request.js:181)
    at Object._readyStateChange (fake_xml_http_request.js:371)
    at Object._setResponseBody (fake_xml_http_request.js:431)
    at Object.respond (fake_xml_http_request.js:452)
    at Object.callback (pretender.js:303)
    at Object.resolve (pretender.js:340)
    at pretender.js:331

I then changed /not-found to 'not-found' (route name instead of a URL, on a hunch), it then errors with the following and enters the parent route:

Uncaught TypeError: Cannot read property 'name' of undefined
Router.isActiveIntent @ ember.debug.js:51337_ember
RuntimeSystemObject.default.extend.isActiveIntent @ ember.debug.js:28405exports.default._ember
RuntimeSystemService.default.extend.isActiveForRoute @ ember.debug.js:24918_ember
ViewsComponentsComponent.default.extend._computeActive @ ember.debug.js:29987
computeLinkToComponentActive @ ember.debug.js:30012ComputedPropertyPrototype.get @ ember.debug.js:14893get @ ember.debug.js:20290
exports.default._emberMetalStreamsStream.default.extend.compute

The 'not-found' route is a glob route:

  this.route('not-found', { path: '/*path' });

Our workaround was to create another route for use in the error action that is not a glob route. This works fine both in route name this.replaceWith('model-not-found') and path form this.replaceWith('/model-not-found').

this.transitionTo('/not-found') works this.transitionTo('not-found') fails with the same error as this.replaceWith('not-found')

I've attempted to recreate the errors here https://github.com/kategengler/recreate-replace-with-errors but was only successful for the this.replaceWith('/not-found') case. For the other case you can see it by pulling down https://github.com/emberobserver/client, installing as usual, changing app/routes/addons/show.js#21 to be this.replaceWith('not-found') and running ember test. It's also possible to see the error when running the app normally but that requires further local setup.

rwjblue commented 8 years ago

Can you test canary? I believe this may be related to a bug that @asakusuma recently fixed.

kategengler commented 8 years ago

Canary fixes the this.replaceWith('/not-found') case!

However the error with this.replaceWith('not-found') still happens.

asakusuma commented 8 years ago

There's a different codepath for a url param vs a route name param, which is probably why one is fixed and the other is not:

https://github.com/emberjs/ember.js/blob/master/packages/ember-routing/lib/system/router.js#L328

stefanpenner commented 8 years ago

However the error with this.replaceWith('not-found') still happens.

Sounds like this will need to be addressed (updating labels)

ghost commented 8 years ago

The errors occurs when you transition from a nested route to a glob route. I.E.: from /addons/xxx to /not-found.

What I'm seeing is that the recognizer of router.js recognizes application, addons and addons.index, while the router's state only has handlerInfos for application and addons.

It then blows up at router.js#L314-L317

seawatts commented 7 years ago

I am still seeing this on ember 2.12.0

mikejerome commented 7 years ago

I ran into this today on ember 2.14.1

No errors when using this.replaceWith('/not-found') but it errors if I leave off the leading slash.

rwjblue commented 7 years ago

I would love a reproduction here. The original repro is pretty out of date (the emberobserver client).

betocantu93 commented 6 years ago

I ran into this, 2.16.2

Within a nested route, within an in-repo-engine, using this.replaceWith('/not-found'); is not possible because of engines and without it, the same TypeError from this issue "Cannot read property 'name'..."

danieledraganti commented 6 years ago

Still the same bug, Ember 2.18. Any fixes on the horizon...?

allthesignals commented 6 years ago

I'm seeing this error too, reproduced here @rwjblue

https://ember-twiddle.com/514033b3906db5bc3193d6f5f9f543c9?openFiles=routes.application.js%2C

I see this error on 3.0 +

pixelhandler commented 6 years ago

@allthesignals @asakusuma @betocantu93 @danidr @kategengler @martndemus @mikejerome @mmun @rwjblue @seawatts @stefanpenner is this still an issue, perhaps we should close or create a new reproduction of this, what do you think?

kalda341 commented 6 years ago

@pixelhandler It is still an issue in 3.4.2.

sandstrom commented 3 years ago

I'm doing some issue gardening 🌱🌿 🌷 and came upon this issue. Since it's quite old I just wanted to ask if this is still relevant? If it isn't, maybe we can close this issue?

By closing some old issues we reduce the list of open issues to a more manageable set.

johanrd commented 3 weeks ago

this is still relevant

NullVoxPopuli commented 3 weeks ago

even in ember 5.12?

johanrd commented 3 weeks ago

Yes.

…Or I at least get the same error TypeError: Cannot read properties of undefined (reading 'name') if the route is a dynamic segment route(path:id), and I try to validate query params in my model hook to then redirect to a fallback query param:

model(params: { id: string }, transition: Transition) {
  if (!['day', 'week', 'month', 'year'].includes(transition?.to?.queryParams?.groupBy)) {
    this.router.replaceWith({ queryParams: { groupBy: 'day' } })
  }
}
johanrd commented 3 weeks ago

@NullVoxPopuli Verified that this is still an issue with ember-source v5.12.0 in the ember-polaris-pokedex app here: https://github.com/johanrd/ember-polaris-pokedex/commit/bee8a143d85f3467756a737507a29ec4d1fef46f#diff-1b1450eb409602d5c7e82920d9919702337ec00c47fb16015e30b8914c39d391R22

For info, a possible workaround seems to be to do replaceWith at a later stage, e.g. setupController