Skip to content

Commit

Permalink
A bunch of fixes based on @ArtskydJ's review
Browse files Browse the repository at this point in the history
  • Loading branch information
TehShrike committed Oct 3, 2017
1 parent aa1a23d commit 97b0dd5
Show file tree
Hide file tree
Showing 11 changed files with 140 additions and 108 deletions.
45 changes: 23 additions & 22 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const StateTransitionManager = require('./lib/state-transition-manager')
const defaultRouterOptions = require('./default-router-options.js')

const series = require('./lib/promise-map-series')
const extend = require('./lib/extend.js')

const denodeify = require('then-denodeify')
const EventEmitter = require('eventemitter3')
Expand All @@ -15,10 +16,10 @@ const combine = require('combine-arrays')
const buildPath = require('page-path-builder')
const nextTick = require('iso-next-tick')

const extend = (...args) => Object.assign({}, ...args)
const property = name => obj => obj[name]
const getProperty = name => obj => obj[name]
const reverse = ary => ary.slice().reverse()
const isFunction = property => obj => typeof obj[property] === 'function'
const isThenable = object => object && (typeof object === 'object' || typeof object === 'function') && typeof object.then === 'function'
const promiseMe = (fn, ...args) => new Promise(resolve => resolve(fn(...args)))

const expectedPropertiesOfAddState = [ 'name', 'route', 'defaultChild', 'data', 'template', 'resolve', 'activate', 'querystringParameters', 'defaultQuerystringParameters', 'defaultParameters' ]
Expand All @@ -30,9 +31,7 @@ module.exports = function StateProvider(makeRenderer, rootElement, stateRouterOp
const stateProviderEmitter = new EventEmitter()
const compareStartAndEndStates = StateComparison(prototypalStateHolder)

const stateNameToArrayofStates = stateName => parse(stateName).map(
name => prototypalStateHolder.get(name)
)
const stateNameToArrayofStates = stateName => parse(stateName).map(prototypalStateHolder.get)

StateTransitionManager(stateProviderEmitter)
const { throwOnError, pathPrefix } = extend({
Expand All @@ -58,7 +57,7 @@ module.exports = function StateProvider(makeRenderer, rootElement, stateRouterOp
function handleError(event, err) {
nextTick(() => {
stateProviderEmitter.emit(event, err)
console.error(event + ' - ' + err.message)
console.error(`${event} - ${err.message}`)
if (throwOnError) {
throw err
}
Expand Down Expand Up @@ -105,7 +104,7 @@ module.exports = function StateProvider(makeRenderer, rootElement, stateRouterOp
content,
template: state.template,
parameters,
}).then(function(newDomApi) {
}).then(newDomApi => {
if (newDomApi) {
activeDomApis[stateName] = newDomApi
}
Expand All @@ -132,7 +131,7 @@ module.exports = function StateProvider(makeRenderer, rootElement, stateRouterOp
}

function renderStateName(parameters, stateName) {
return getChildElementForStateName(stateName).then(function(childElement) {
return getChildElementForStateName(stateName).then(element => {
const state = prototypalStateHolder.get(stateName)
const content = getContentObject(activeStateResolveContent, stateName)

Expand All @@ -143,8 +142,8 @@ module.exports = function StateProvider(makeRenderer, rootElement, stateRouterOp
})

return renderDom({
element: childElement,
template: state.template,
element,
content,
parameters,
}).then(domApi => {
Expand Down Expand Up @@ -191,15 +190,15 @@ module.exports = function StateProvider(makeRenderer, rootElement, stateRouterOp

function addState(state) {
if (typeof state === 'undefined') {
throw new Error('Expected \'state\' to be passed in.')
throw new Error(`Expected 'state' to be passed in.`)
} else if (typeof state.name === 'undefined') {
throw new Error('Expected the \'name\' option to be passed in.')
throw new Error(`Expected the 'name' option to be passed in.`)
} else if (typeof state.template === 'undefined') {
throw new Error('Expected the \'template\' option to be passed in.')
throw new Error(`Expected the 'template' option to be passed in.`)
}
Object.keys(state).filter(function(key) {
Object.keys(state).filter(key => {
return expectedPropertiesOfAddState.indexOf(key) === -1
}).forEach(function(key) {
}).forEach(key => {
console.warn('Unexpected property passed to addState:', key)
})

Expand All @@ -224,7 +223,7 @@ module.exports = function StateProvider(makeRenderer, rootElement, stateRouterOp
function ifNotCancelled(fn) {
return (...args) => {
if (transition.cancelled) {
const err = new Error('The transition to ' + newStateName + 'was cancelled')
const err = new Error(`The transition to ${newStateName} was cancelled`)
err.wasCancelledBySomeoneElse = true
throw err
} else {
Expand All @@ -250,10 +249,11 @@ module.exports = function StateProvider(makeRenderer, rootElement, stateRouterOp
lastStateStartedActivating.set(state.name, parameters)
})).then(function getStateChanges() {
const stateComparisonResults = compareStartAndEndStates({
originalState: lastCompletelyLoadedState.get().name,
originalParameters: lastCompletelyLoadedState.get().parameters,
newState: newStateName,
newParameters: parameters,
original: lastCompletelyLoadedState.get(),
destination: {
name: newStateName,
parameters,
},
})
return stateChangeLogic(stateComparisonResults) // { destroy, change, create }
}).then(ifNotCancelled(function resolveDestroyAndActivateStates(stateChanges) {
Expand Down Expand Up @@ -353,7 +353,8 @@ module.exports = function StateProvider(makeRenderer, rootElement, stateRouterOp
options = extend(defaultOptions, options)
const goFunction = options.replace ? router.replace : router.go

return promiseMe(makePath, newStateName, parameters, options).then(goFunction, handleError.bind(null, 'stateChangeError'))
return promiseMe(makePath, newStateName, parameters, options)
.then(goFunction, err => handleError('stateChangeError', err))
}
stateProviderEmitter.evaluateCurrentRoute = (defaultState, defaultParams) => {
return promiseMe(makePath, defaultState, defaultParams).then(defaultPath => {
Expand Down Expand Up @@ -404,7 +405,7 @@ function redirector(newStateName, parameters) {
// { [stateName]: resolveResult }
function resolveStates(states, parameters) {
const statesWithResolveFunctions = states.filter(isFunction('resolve'))
const stateNamesWithResolveFunctions = statesWithResolveFunctions.map(property('name'))
const stateNamesWithResolveFunctions = statesWithResolveFunctions.map(getProperty('name'))

const resolves = Promise.all(statesWithResolveFunctions.map(state => {
return new Promise((resolve, reject) => {
Expand All @@ -415,7 +416,7 @@ function resolveStates(states, parameters) {
}

const res = state.resolve(state.data, parameters, resolveCb)
if (res && (typeof res === 'object' || typeof res === 'function') && typeof res.then === 'function') {
if (isThenable(res)) {
resolve(res)
}
})
Expand Down
1 change: 1 addition & 0 deletions lib/extend.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = (...args) => Object.assign({}, ...args)
4 changes: 1 addition & 3 deletions lib/promise-map-series.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ module.exports = function sequence(array, iterator) {
let currentPromise = Promise.resolve()
return Promise.all(
array.map((value, i) => {
const thisPromiseResult = currentPromise.then(() => iterator(value, i, array))
currentPromise = thisPromiseResult
return thisPromiseResult
return currentPromise = currentPromise.then(() => iterator(value, i, array))
})
)
}
16 changes: 9 additions & 7 deletions lib/state-comparison.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
const stateStringParser = require('./state-string-parser')
const extend = require('./extend.js')

const combine = require('combine-arrays')
const pathToRegexp = require('path-to-regexp-with-reversible-keys')

module.exports = function StateComparison(stateState) {
const getPathParameters = pathParameters()

const parametersChanged = args => parametersThatMatterWereChanged(Object.assign({}, args, { stateState, getPathParameters }))
const parametersChanged = args => parametersThatMatterWereChanged(extend(args, { stateState, getPathParameters }))

return args => stateComparison(Object.assign({}, args, { parametersChanged }))
return args => stateComparison(extend(args, { parametersChanged }))
}

function pathParameters() {
Expand Down Expand Up @@ -38,10 +40,10 @@ function parametersThatMatterWereChanged({ stateState, getPathParameters, stateN
)
}

function stateComparison({ parametersChanged, originalState, originalParameters, newState, newParameters }) {
function stateComparison({ parametersChanged, original, destination }) {
const states = combine({
start: stateStringParser(originalState),
end: stateStringParser(newState),
start: stateStringParser(original.name),
end: stateStringParser(destination.name),
})

return states.map(({ start, end }) => ({
Expand All @@ -50,8 +52,8 @@ function stateComparison({ parametersChanged, originalState, originalParameters,
stateNameChanged: start !== end,
stateParametersChanged: start === end && parametersChanged({
stateName: start,
fromParameters: originalParameters,
toParameters: newParameters,
fromParameters: original.parameters,
toParameters: destination.parameters,
}),
}))
}
2 changes: 1 addition & 1 deletion lib/state-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = function StateState() {

return names.map(name => {
if (!states[name]) {
throw new Error('State ' + name + ' not found')
throw new Error(`State ${name} not found`)
}
return states[name]
})
Expand Down
2 changes: 1 addition & 1 deletion rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import visualizer from 'rollup-plugin-visualizer'
import { list as babelHelpersList } from 'babel-helpers'

export default {
name: 'revelationStructure',
name: 'abstractStateRouter',
input: './index.js',
output: {
file: './bundle.js',
Expand Down
4 changes: 2 additions & 2 deletions test/default-child.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ function RememberActivation(location) {
}
}
return {
activate: activate,
onEnd: onEnd,
activate,
onEnd,
}
}

Expand Down
8 changes: 4 additions & 4 deletions test/dom-interaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@ test('All dom functions called in order', function(t) {
render: function render(context, cb) {
const element = context.element
const template = context.template
actions.push('render ' + template + ' on ' + element)
actions.push(`render ${template} on ${element}`)
cb(null, template)
},
reset: function reset(context, cb) {
actions.push('reset ' + context.domApi)
actions.push(`reset ${context.domApi}`)
cb()
},
destroy: function destroy(renderedTemplateApi, cb) {
actions.push('destroy ' + renderedTemplateApi)
actions.push(`destroy ${renderedTemplateApi}`)
cb()
},
getChildElement: function getChildElement(renderedTemplateApi, cb) {
actions.push('getChild ' + renderedTemplateApi)
actions.push(`getChild ${renderedTemplateApi}`)
cb(null, renderedTemplateApi + ' child')
},
}
Expand Down
Loading

1 comment on commit 97b0dd5

@ArtskydJ
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Please sign in to comment.