Skip to content

Commit 7744eae

Browse files
committed
fix(lazyLoad): abort URL-sourced transition when lazy loaded states do not match the URL
When a URL matches a future state's prefix but nothing in the lazy loaded module matches the full URL, retryTransition() delegates to urlService.sync() but returned undefined, letting the original transition resume and activate the deregistered .** placeholder, superseding the otherwise() transition started by sync(). Return false after sync() to abort the original transition so the sync()-initiated transition (e.g. the otherwise() rule) can complete. Closes ui-router#850
1 parent c647c9c commit 7744eae

2 files changed

Lines changed: 54 additions & 1 deletion

File tree

src/hooks/lazyLoad.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ const lazyLoadHook: TransitionHookFn = (transition: Transition) => {
5656

5757
// No matching state found, so let .sync() choose the best non-state match/otherwise
5858
router.urlService.sync();
59+
// Abort this transition; it would supersede the transition started by sync()
60+
// and activate the (now deregistered) future state placeholder (issue #850)
61+
return false;
5962
}
6063

6164
const promises = transition

test/lazyLoadSpec.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { UIRouter, TransitionService, StateService } from '../src/index';
1+
import { UIRouter, TransitionService, StateService, RejectType, Rejection } from '../src/index';
22

33
import { StateRegistry } from '../src/state/stateRegistry';
44
import { UrlRouter } from '../src/url/urlRouter';
@@ -298,6 +298,56 @@ describe('future state', function() {
298298
});
299299
});
300300

301+
// Tests for https://github.com/ui-router/core/issues/850
302+
describe('triggered by a URL sync that the lazy loaded states do not match', () => {
303+
let lazyLoadCount: number;
304+
305+
beforeEach(() => {
306+
lazyLoadCount = 0;
307+
$state.defaultErrorHandler(() => {});
308+
$registry.register({ name: 'notFound' });
309+
router.urlService.rules.otherwise({ state: 'notFound' });
310+
$registry.register({
311+
name: 'A.**',
312+
url: '/a',
313+
lazyLoad: () => {
314+
lazyLoadCount++;
315+
return Promise.resolve({ states: [{ name: 'A', url: '/a/:id' }] });
316+
},
317+
});
318+
});
319+
320+
it('should activate the otherwise() rule instead of the deregistered placeholder', (done) => {
321+
router.urlService.url('/a/no/such/page');
322+
router.urlService.sync();
323+
324+
$transitions.onSuccess({}, () => {
325+
expect(lazyLoadCount).toBe(1);
326+
expect($state.current.name).toBe('notFound');
327+
expect($registry.get('A.**')).toBeFalsy();
328+
done();
329+
});
330+
});
331+
332+
it('should abort the original transition to the deregistered placeholder', (done) => {
333+
const errors: { targetName: string; error: Rejection }[] = [];
334+
$transitions.onError({}, (trans) => {
335+
errors.push({ targetName: trans.targetState().name(), error: trans.error() });
336+
});
337+
338+
// Only change the url; the url listener syncs it (avoids starting a second, duplicate transition)
339+
router.urlService.url('/a/no/such/page');
340+
341+
$transitions.onSuccess({}, () => {
342+
expect(errors.length).toBe(1);
343+
expect(errors[0].targetName).toBe('A.**');
344+
expect(errors[0].error.type).toBe(RejectType.ABORTED);
345+
expect($state.current.name).toBe('notFound');
346+
done();
347+
});
348+
});
349+
});
350+
301351
describe('that resolves to multiple states', () => {
302352
let futureA, futureB;
303353
const lazyA = { name: 'A', url: '/a/:id', params: { id: 'default' } };

0 commit comments

Comments
 (0)