Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-async resolve results in confusing behavior #150

Open
ArtskydJ opened this issue Jul 5, 2022 · 1 comment
Open

Non-async resolve results in confusing behavior #150

ArtskydJ opened this issue Jul 5, 2022 · 1 comment

Comments

@ArtskydJ
Copy link
Collaborator

ArtskydJ commented Jul 5, 2022

I put some test data in the resolve, and forgot to set it as an async function. When I try to load a route, I don't get any error message or anything. It just appears to get stuck.

resolve: () => {
    const thing = {
        test_data: true
    }
    return thing
}

I would have expected it to either work as-is, or to throw an error about a missing .then method.

@TehShrike
Copy link
Owner

So ASR was originally written in the error-first callback style, and didn't support promises. Promise support was added later.

To maintain backwards compatibility, the callback style is still supported. The way that works is that if the resolve function returns a thenable, ASR internally resolves with the result of that thenable. Otherwise, it expects that the user plans to call the callback function in the future.

const resolves = Promise.all(statesWithResolveFunctions.map(state => new Promise((resolve, reject) => {
const resolveCb = (err, content) => err ? reject(err) : resolve(content)
resolveCb.redirect = (newStateName, parameters) => {
reject(redirector(newStateName, parameters))
}
const res = state.resolve(state.data, parameters, resolveCb)
if (isThenable(res)) {
resolve(res)
}
})))

At this point I'm fine with publishing the breaking change that removes support for callback functions and only exposes the promise API. Callbacks have been dead for a long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants