Promise based initializers #572

mehulkar · 2020-01-10T01:09:43Z

NullVoxPopuli · 2020-01-10T01:15:42Z

text/0000-promise-based-initializers.md

+before the application begins routing.
+
+For example, when booting an Ember application in an embedded environment, it may be necessary to
+listen for events from the environment to determine the initial state to determine the initial URL.


would using the beforeModel in the application route work?

For this particular use case, the application route is too late because there are subtle differences between what it means to start routing at a URL and what it means to redirect to a URL. When an Ember app runs in a browser, it does the former.

That said, I think promise based initializers will have benefits beyond just determining the initial state of the application.

You can't redirect from the application route though. It,s the parent of everything.

redirecting is just a fancy word for saying " cancel the current transition and start a new one". You can intercept a transition in the ApplicationRoute that would have ended at /foo and and redirect to /bar, for example.

kpfefferle · 2020-01-10T19:01:27Z

text/0000-promise-based-initializers.md

+
+Currently, the earliest place that developers can write asynchronous code is in the ApplicationRoute.
+For some use cases, this is not early enough, and it is useful to be able to run asynchronous code
+before the application begins routing.


Do we have known use cases where the Application route "is not early enough"? I'm not doubting the truth of this statement, but having an example use case or cases would make it easier for me to wrap my head around how this would be useful.

Sure! If you have a webview embedded in a native layer that sends you events (say when the native app is launched via deeplink). Capturing that event and operating on it is an asynchronous operation. If you wait till the ApplicaitonRoute, the Ember app has already started routing, so the only option to get to the desired route is redirect(). That's starkly different from what happens when the Ember app is initialized at a subpath in the browser, however and it can cause redirect hell if you receive more than one of these events.

Even accepting this case as gospel, I don't see why the changes proposed in this RFC would be needed. The specific case seems to be that you want to avoid autobooting your application (until these events have come back), that is already a supported operation (without changes AFAICT).

This feels to me to be introducing a new feature to Ember for what seems like a very very specific/targeted scenario for a single application (that already has argubly better solutions).

skipping autoboot is very much NOT the happy path, regardless of what the intention of the visit API was. The reason I introduced this API is because initializers are one of the few non-async code paths in Ember APIs today and it seems like a good thing to have them regardless of the use cases I've listed here.

We do have other use cases also:

Making a network call that sets some cookies before we do anything else

Initializing 3rd libraries that have async APIs (or make their own async calls)

This stuff can happen in the route, but I'd argue that not all of them should happen in a route, if we have the concept of an initializer. If initializers weren't a thing at all, I'd focus more on the manual boot scenario and improving that.

Gaurav0 · 2020-01-11T17:40:22Z

Really like this!

Could we add an afterAsync key or something like that so that one initializer could wait for another one to resolve?

xg-wang · 2020-01-12T06:03:50Z

text/0000-promise-based-initializers.md

+implements a method that loops through the loaded initializers and executes the exported function.
+
+Initializer and Instance Initializer files each export an object with an `initialize` property
+that references a function. For backwards compatibility, an `async` property should be added, and


If the initialize return type can be detected do we still need async? I think handling async initializers based on this option or the actual return type affects unresolved questions below.

Good call out. I debated over the need for this key too. The main reason it's useful is that you cannot detect the return type without running the initializer. The purpose of knowing earlier is to opt the user into the promise-based flow (the whole thing would need to be wrapped in a promise). Without any async declarations, existing apps would be able to retain the exact same behavior as they have today.

NullVoxPopuli · 2020-01-12T13:54:52Z

I'd be more of a fan of deprecating intializers as a whole. I don't think they're needed.

Ime, everything initializers can do is better handled in the application route. Either in a before model, or the init/constructor.

mehulkar · 2020-01-12T20:25:05Z

I'd be more of a fan of deprecating intializers as a whole. I don't think they're needed.

Few thoughts:

The app/initializers and app/instance-initilalizers directories can definitely go away IMO. App Boot Hooks #573 is my proposal for that.
Whether or not "code run early in the application lifecycle" is unnecessary is debatable. I've heard the argument for "just put everything in the ApplicationRoute#beforeModel" argument, and I'm not sure how I feel about it. To some extent, my opinion is "who cares what you call it, as long as it runs at the right time", but I also feel like it is useful to indicate that some code needs to run at app initialization because it has nothing to do any of the routing. I find that stuffing code into ApplicationRoute#beforeModel is similar to having a top-level wrapper component in React that does a bunch of stuff, only because there's no other place to do it.

In any case, I think removing initializers completely is a bit out of scope here and would require a lot more discussion and feedback from the community. This seems like a pretty easy win that would put us on the path to a fully async app boot, and then we can fiddle around with where code lives.

mehulkar · 2020-01-12T20:27:03Z

Could we add an afterAsync key or something like that so that one initializer could wait for another one to resolve?

I think the existing before/after keys would still be expected to work. Once you add one async initializer, you're forced to wrap all initializers in a promise. And since the ordering is done before the initializers run anyway, these existing keys would work. Let me know if I should add something to the text of the RFC to make that more clear.

As a side note, I think the better solution to ordering is the approach in #573, rather than continuing down the before/after path.

rwjblue

As I mentioned in one of the other RFCs (#573 I think?), I'm personally pretty opposed to making any changes to the initializer system that are not directly targeted at making its removal easier.

rwjblue · 2020-02-07T15:45:44Z

text/0000-promise-based-initializers.md

+Another example may be to interact with 3rd party libraries that need to be initialized early but
+have an asynchronous API.
+
+## Detailed Design


Some important details (to me at least 😸) are not mentioned here:

What happens if you say you are after an intializer that is async, are you called after its promise resolves? The RFC should explicitly state this behavior.

What is the benefit of adding async: true (vs doing something like promise-map-series)?

yes

async: true was dreamed up for opt-in beahvior and to make it easier to sell :D. I don't know what promise-map-series is, but it sounds like wrapping each initializer in a promise and executing them in order? i'm also fine with that, but I was worried that could have performance implications or could make it possible for some other framework code microtasks to run in between initializers. If neither of those are concerns, I have no problems with it.

rwjblue · 2020-02-07T15:49:29Z

text/0000-promise-based-initializers.md

+
+Currently, the earliest place that developers can write asynchronous code is in the ApplicationRoute.
+For some use cases, this is not early enough, and it is useful to be able to run asynchronous code
+before the application begins routing.


Even accepting this case as gospel, I don't see why the changes proposed in this RFC would be needed. The specific case seems to be that you want to avoid autobooting your application (until these events have come back), that is already a supported operation (without changes AFAICT).

This feels to me to be introducing a new feature to Ember for what seems like a very very specific/targeted scenario for a single application (that already has argubly better solutions).

rwjblue · 2020-02-07T15:50:49Z

text/0000-promise-based-initializers.md

+should default to `false` if omitted. If true (or truthy), the Application/ApplicationInstance would
+expect a promise to be returned from the function and wait for the promise to resolve
+before running the next initializer in the queue. The value resolved from the promise would be
+ignored. Promises that reject would throw an exception in development and be caught in production


and be caught in production

What does this mean? If initialization of an application fails, I cannot reason about how to make that app functional again...

it's up to do the developer to determine that. For example, we make a network request at the beginning to set some cookies. Without those cookies the app is perfectly functional, but could have unintended consequences later. Catching errors in production would just be a failsafe for apps who opts into this feature, but doesn't think through that. I'd be fine with removing this part.

rwjblue · 2020-02-07T15:51:32Z

text/0000-promise-based-initializers.md

+If implemented, future RFCs could:
+
+- change the default value of the `async` property to `true`.
+- deprecate `deferReadiness` and `advanceReadiness` entirely.


👍👍 - Seems like a great thing to do when we deprecate initializers 😃

👍. It's up to core team what order to do all this in. This RFC seemed like a good path to "just use JS" that didn't require changing the initializer paradigm!

mehulkar · 2020-06-08T19:02:40Z

@chancancode do you have any thoughts here (considering you did the seminal work for async boot in emberjs/ember.js@1a42386)?

mehulkar · 2020-08-06T23:18:34Z

After talking with @rwjblue and @pzuraq, I think it makes more sense to pursue #573 instead of this. Moving away from initializers will be better for debugability, more explicit code, and some bundle size improvements. This RFC could probably move to FCP to close!

mehulkar · 2020-08-06T23:30:46Z

Closing this down!

Mehul Kar added 2 commits January 9, 2020 17:08

Promise based initializers

9b317d7

Add PR link

dd16ce7

NullVoxPopuli reviewed Jan 10, 2020

View reviewed changes

kpfefferle reviewed Jan 10, 2020

View reviewed changes

mehulkar mentioned this pull request Jan 10, 2020

App Boot Hooks #573

Closed

jayjayjpg mentioned this pull request Jan 10, 2020

The Ember Times No. 131 - January 17th 2020 ember-learn/ember-blog#458

Merged

28 tasks

xg-wang reviewed Jan 12, 2020

View reviewed changes

amyrlam mentioned this pull request Jan 17, 2020

The Ember Times No. 132 - January 24th 2020 ember-learn/ember-blog#498

Merged

25 tasks

amyrlam mentioned this pull request Jan 24, 2020

The Ember Times No. 133 - January 31st 2020 ember-learn/ember-blog#505

Merged

27 tasks

rwjblue added the T-framework RFCs that impact the ember.js library label Jan 31, 2020

rwjblue reviewed Feb 7, 2020

View reviewed changes

attempt to better describe motivations

f1c8aa9

mehulkar closed this Aug 6, 2020

mehulkar deleted the promise-initializers branch August 6, 2020 23:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Promise based initializers #572

Promise based initializers #572

mehulkar commented Jan 10, 2020 •

edited

Loading

NullVoxPopuli Jan 10, 2020

mehulkar Jan 10, 2020 •

edited

Loading

NullVoxPopuli Jan 12, 2020

mehulkar Feb 4, 2020

kpfefferle Jan 10, 2020

mehulkar Jan 10, 2020

rwjblue Feb 7, 2020

mehulkar Feb 8, 2020

Gaurav0 commented Jan 11, 2020

xg-wang Jan 12, 2020

mehulkar Jan 12, 2020

NullVoxPopuli commented Jan 12, 2020

mehulkar commented Jan 12, 2020

mehulkar commented Jan 12, 2020

rwjblue left a comment

rwjblue Feb 7, 2020

mehulkar Feb 8, 2020

rwjblue Feb 7, 2020

rwjblue Feb 7, 2020

mehulkar Feb 8, 2020

rwjblue Feb 7, 2020

mehulkar Feb 8, 2020

mehulkar commented Jun 8, 2020

mehulkar commented Aug 6, 2020

mehulkar commented Aug 6, 2020

Promise based initializers #572

Promise based initializers #572

Conversation

mehulkar commented Jan 10, 2020 • edited Loading

Choose a reason for hiding this comment

mehulkar Jan 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gaurav0 commented Jan 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NullVoxPopuli commented Jan 12, 2020

mehulkar commented Jan 12, 2020

mehulkar commented Jan 12, 2020

rwjblue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mehulkar commented Jun 8, 2020

mehulkar commented Aug 6, 2020

mehulkar commented Aug 6, 2020

mehulkar commented Jan 10, 2020 •

edited

Loading

mehulkar Jan 10, 2020 •

edited

Loading