Skip to content

Commit

Permalink
fix: add better checking against undefined
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexlafroscia committed Aug 28, 2018
1 parent e4911a9 commit 9e88557
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 17 deletions.
8 changes: 4 additions & 4 deletions addon/-private/state-machine/-base.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import EmberObject, { set } from '@ember/object';
import MutableArray from '@ember/array/mutable';
import { isBlank, isPresent } from '@ember/utils';
import { A } from '@ember/array';
import { readOnly } from '@ember-decorators/object/computed';
import { assert } from '@ember/debug';
import { isNone } from '@ember/utils';

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

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

if (isBlank(this.currentStep)) {
if (typeof this.currentStep === 'undefined') {
set(this, 'currentStep', name);
}
}
Expand Down Expand Up @@ -67,7 +67,7 @@ export default abstract class BaseStateMachine extends EmberObject {
activate(step: StepNode | StepName) {
const name = step instanceof StepNode ? step.name : step;

assert('No step name was provided', isPresent(step));
assert('No step name was provided', !isNone(step));
assert(
`"${name}" does not match an existing step`,
this.stepTransitions.map(node => node.name).includes(name)
Expand Down
24 changes: 15 additions & 9 deletions addon/components/step-manager.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import Component from '@ember/component';
// @ts-ignore: Ignore import of compiled template
import layout from '../templates/components/step-manager';
import { get, set } from '@ember/object';
import { isEmpty, isPresent } from '@ember/utils';
import { get, getProperties, set } from '@ember/object';
import { isPresent, isNone } from '@ember/utils';
import { schedule } from '@ember/runloop';
import { assert } from '@ember/debug';
import { action, computed } from '@ember-decorators/object';
Expand Down Expand Up @@ -77,7 +77,13 @@ export default class StepManagerComponent extends Component {
constructor() {
super(...arguments);

const initialStep = get(this, 'initialStep') || get(this, 'currentStep');
const { initialStep, currentStep } = getProperties(
this,
'initialStep',
'currentStep'
);

const startingStep = isNone(initialStep) ? currentStep : initialStep;

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

set(this, 'transitions', new StateMachine(initialStep));
set(this, 'transitions', new StateMachine(startingStep));
}

@computed('transitions.{currentStep,length}')
get hasNextStep() {
return isPresent(this.transitions.pickNext());
return !isNone(this.transitions.pickNext());
}

@computed('transitions.{currentStep,length}')
get hasPreviousStep() {
return isPresent(this.transitions.pickPrevious());
return !isNone(this.transitions.pickPrevious());
}

didUpdateAttrs() {
Expand Down Expand Up @@ -146,7 +152,7 @@ export default class StepManagerComponent extends Component {

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

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

assert('There is no next step', !!to);
assert('There is no next step', !isNone(to));

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

assert('There is no previous step', !!to);
assert('There is no previous step', !isNone(to));

this.transitionTo(to!);
}
Expand Down
9 changes: 5 additions & 4 deletions addon/components/step-manager/step.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Component from '@ember/component';
// @ts-ignore: Ignore import of compiled template
import layout from '../../templates/components/step-manager/step';
import { get, set } from '@ember/object';
import { isEmpty, isPresent } from '@ember/utils';
import { isPresent } from '@ember/utils';
import { assert } from '@ember/debug';
import { computed } from '@ember-decorators/object';
import { tagName } from '@ember-decorators/component';
Expand Down Expand Up @@ -33,9 +33,10 @@ export default class StepComponent extends Component {
super(...arguments);

const nameAttribute = get(this, 'name');
const name = isEmpty(nameAttribute)
? Symbol('generated step name')
: nameAttribute;
const name =
typeof nameAttribute === 'undefined'
? Symbol('generated step name')
: nameAttribute;

assert('Step name cannot be a boolean', typeof name !== 'boolean');
assert('Step name cannot be an object', typeof name !== 'object');
Expand Down
40 changes: 40 additions & 0 deletions tests/integration/step-manager-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -840,4 +840,44 @@ module('step-manager', function(hooks) {
.exists('The initial step is still visible');
});
});

module('edge cases', function() {
test('it handles steps with falsy names', async function(assert) {
await render(hbs`
{{#step-manager initialStep='' as |w|}}
{{#w.step name=''}}
<div data-test-empty-string></div>
{{/w.step}}
{{#w.step name=0}}
<div data-test-zero></div>
{{/w.step}}
<button {{action w.transition-to-previous}} data-test-previous>
Previous step
</button>
<button {{action w.transition-to-next}} data-test-next>
Next step
</button>
{{/step-manager}}
`);

assert
.dom('[data-test-empty-string]')
.exists('Can start on a step with a falsy name');

await click('[data-test-next]');

assert
.dom('[data-test-zero]')
.exists('Can transition to a next step with a falsy name');

await click('[data-test-previous]');

assert
.dom('[data-test-empty-string]')
.exists('Can transition to a previous step with a falsy name');
});
});
});

0 comments on commit 9e88557

Please sign in to comment.