Skip to content

Commit 9e88557

Browse files
committed
fix: add better checking against undefined
The first pass at checking explicitly for `undefined`, rather than just a falsy value, was not enough. This adds a better test case and fixes a number of issues I found while getting it to pass. Closes #112
1 parent e4911a9 commit 9e88557

File tree

4 files changed

+64
-17
lines changed

4 files changed

+64
-17
lines changed

addon/-private/state-machine/-base.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import EmberObject, { set } from '@ember/object';
22
import MutableArray from '@ember/array/mutable';
3-
import { isBlank, isPresent } from '@ember/utils';
43
import { A } from '@ember/array';
54
import { readOnly } from '@ember-decorators/object/computed';
65
import { assert } from '@ember/debug';
6+
import { isNone } from '@ember/utils';
77

88
import { StepName } from '../types';
99
import StepNode from '../step-node';
@@ -28,7 +28,7 @@ export default abstract class BaseStateMachine extends EmberObject {
2828
constructor(initialStepName?: StepName) {
2929
super();
3030

31-
if (isPresent(initialStepName)) {
31+
if (!isNone(initialStepName)) {
3232
set(this, 'currentStep', initialStepName!);
3333
}
3434
}
@@ -37,7 +37,7 @@ export default abstract class BaseStateMachine extends EmberObject {
3737
const node = new StepNode(this, name, context);
3838
this.stepTransitions.pushObject(node);
3939

40-
if (isBlank(this.currentStep)) {
40+
if (typeof this.currentStep === 'undefined') {
4141
set(this, 'currentStep', name);
4242
}
4343
}
@@ -67,7 +67,7 @@ export default abstract class BaseStateMachine extends EmberObject {
6767
activate(step: StepNode | StepName) {
6868
const name = step instanceof StepNode ? step.name : step;
6969

70-
assert('No step name was provided', isPresent(step));
70+
assert('No step name was provided', !isNone(step));
7171
assert(
7272
`"${name}" does not match an existing step`,
7373
this.stepTransitions.map(node => node.name).includes(name)

addon/components/step-manager.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import Component from '@ember/component';
22
// @ts-ignore: Ignore import of compiled template
33
import layout from '../templates/components/step-manager';
4-
import { get, set } from '@ember/object';
5-
import { isEmpty, isPresent } from '@ember/utils';
4+
import { get, getProperties, set } from '@ember/object';
5+
import { isPresent, isNone } from '@ember/utils';
66
import { schedule } from '@ember/runloop';
77
import { assert } from '@ember/debug';
88
import { action, computed } from '@ember-decorators/object';
@@ -77,7 +77,13 @@ export default class StepManagerComponent extends Component {
7777
constructor() {
7878
super(...arguments);
7979

80-
const initialStep = get(this, 'initialStep') || get(this, 'currentStep');
80+
const { initialStep, currentStep } = getProperties(
81+
this,
82+
'initialStep',
83+
'currentStep'
84+
);
85+
86+
const startingStep = isNone(initialStep) ? currentStep : initialStep;
8187

8288
if (!isPresent(this.linear)) {
8389
this.linear = true;
@@ -87,17 +93,17 @@ export default class StepManagerComponent extends Component {
8793
? LinearStateMachine
8894
: CircularStateMachine;
8995

90-
set(this, 'transitions', new StateMachine(initialStep));
96+
set(this, 'transitions', new StateMachine(startingStep));
9197
}
9298

9399
@computed('transitions.{currentStep,length}')
94100
get hasNextStep() {
95-
return isPresent(this.transitions.pickNext());
101+
return !isNone(this.transitions.pickNext());
96102
}
97103

98104
@computed('transitions.{currentStep,length}')
99105
get hasPreviousStep() {
100-
return isPresent(this.transitions.pickPrevious());
106+
return !isNone(this.transitions.pickPrevious());
101107
}
102108

103109
didUpdateAttrs() {
@@ -146,7 +152,7 @@ export default class StepManagerComponent extends Component {
146152

147153
// If `currentStep` is present, it's probably something the user wants
148154
// two-way-bound with the new value
149-
if (!isEmpty(this.currentStep)) {
155+
if (!isNone(this.currentStep)) {
150156
set(this, 'currentStep', destination);
151157
}
152158

@@ -157,7 +163,7 @@ export default class StepManagerComponent extends Component {
157163
transitionToNext() {
158164
const to = this.transitions.pickNext();
159165

160-
assert('There is no next step', !!to);
166+
assert('There is no next step', !isNone(to));
161167

162168
this.transitionTo(to!);
163169
}
@@ -166,7 +172,7 @@ export default class StepManagerComponent extends Component {
166172
transitionToPrevious() {
167173
const to = this.transitions.pickPrevious();
168174

169-
assert('There is no previous step', !!to);
175+
assert('There is no previous step', !isNone(to));
170176

171177
this.transitionTo(to!);
172178
}

addon/components/step-manager/step.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import Component from '@ember/component';
22
// @ts-ignore: Ignore import of compiled template
33
import layout from '../../templates/components/step-manager/step';
44
import { get, set } from '@ember/object';
5-
import { isEmpty, isPresent } from '@ember/utils';
5+
import { isPresent } from '@ember/utils';
66
import { assert } from '@ember/debug';
77
import { computed } from '@ember-decorators/object';
88
import { tagName } from '@ember-decorators/component';
@@ -33,9 +33,10 @@ export default class StepComponent extends Component {
3333
super(...arguments);
3434

3535
const nameAttribute = get(this, 'name');
36-
const name = isEmpty(nameAttribute)
37-
? Symbol('generated step name')
38-
: nameAttribute;
36+
const name =
37+
typeof nameAttribute === 'undefined'
38+
? Symbol('generated step name')
39+
: nameAttribute;
3940

4041
assert('Step name cannot be a boolean', typeof name !== 'boolean');
4142
assert('Step name cannot be an object', typeof name !== 'object');

tests/integration/step-manager-test.js

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,4 +840,44 @@ module('step-manager', function(hooks) {
840840
.exists('The initial step is still visible');
841841
});
842842
});
843+
844+
module('edge cases', function() {
845+
test('it handles steps with falsy names', async function(assert) {
846+
await render(hbs`
847+
{{#step-manager initialStep='' as |w|}}
848+
{{#w.step name=''}}
849+
<div data-test-empty-string></div>
850+
{{/w.step}}
851+
852+
{{#w.step name=0}}
853+
<div data-test-zero></div>
854+
{{/w.step}}
855+
856+
<button {{action w.transition-to-previous}} data-test-previous>
857+
Previous step
858+
</button>
859+
860+
<button {{action w.transition-to-next}} data-test-next>
861+
Next step
862+
</button>
863+
{{/step-manager}}
864+
`);
865+
866+
assert
867+
.dom('[data-test-empty-string]')
868+
.exists('Can start on a step with a falsy name');
869+
870+
await click('[data-test-next]');
871+
872+
assert
873+
.dom('[data-test-zero]')
874+
.exists('Can transition to a next step with a falsy name');
875+
876+
await click('[data-test-previous]');
877+
878+
assert
879+
.dom('[data-test-empty-string]')
880+
.exists('Can transition to a previous step with a falsy name');
881+
});
882+
});
843883
});

0 commit comments

Comments
 (0)