Skip to content

Auto autoboot#12733

Closed
eriktrom wants to merge 3 commits intoemberjs:masterfrom
eriktrom:default-autoboot-false-in-node
Closed

Auto autoboot#12733
eriktrom wants to merge 3 commits intoemberjs:masterfrom
eriktrom:default-autoboot-false-in-node

Conversation

@eriktrom
Copy link
Contributor

Make autoboot setting actually automatic

cc @tomdale @chancancode

The code paths inside Application.init, within the ember-application-visit flag
are never hit by any of the tests. Furthermore, autoboot must be explicitly set
to false when running in node to allow passing in a url when booting an app
instance.

Thus, make it default to false when in node, true when in the browser, to
abstract this concern away from the user.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is regression b/c ?

also, although need to run and make sample app to show it, should work
with the following commits to the officially supported ember fastboot
build and server repos:

- https://github.com/eriktrom/ember-cli-fastboot/tree/one-build
- https://github.com/eriktrom/ember-fastboot-server/tree/one-build
@eriktrom
Copy link
Contributor Author

@tomdale - as of last commit, this works fully

here's the benefit, from your build and server repos:

eriktrom/ember-cli-fastboot@master...one-build
eriktrom/ember-fastboot-server@master...one-build

i know it would sure make my build similarly simpler as well. also seems to make fastboot officially supported build being more prone to being agnostic to backend server framework (at some point) and even makes it easier to separate your fastboot server repo from ember cli as well (cause now our builds look almost the same, and im not using ember cli middleware at all, using hapi).

let me know of your interest level and i'll make a sample app to prove this works with your repos (the code in those links i haven't actually ran yet :P, but works on my setup, they are very similar now)

note that the incrementAutoboot flag in the last commit would be temporary, its just there for the tests that i think need some more thought, 10 or 11 of them testing that you can set the autoboot setting to false is what that hack is (still far less than the build setup though if we're counting)

failure for ci is saucelabs - seems stuck today

in any case, i had fun and learned a lot so we can close and re-visit later, no biggie, i know we never talked about this, it was just annoying me :)

caveats - none, you can still set the autoboot flag from Ember.Application namespace, when calling create, but doing so requires leaving some form of the incrementAutoboot method in place - although i'd be willing to bet there is a much cleaner way if we decide to continue with this thought process, which in general should clean things up that seem hacky/cruft as they are

EDIT: 1 time for grammar and caveats

@eriktrom eriktrom changed the title Default autoboot to false when in node environment Auto autoboot Dec 22, 2015
@homu
Copy link
Contributor

homu commented Mar 19, 2016

☔ The latest upstream changes (presumably #13100) made this pull request unmergeable. Please resolve the merge conflicts.

@eriktrom
Copy link
Contributor Author

@homu - I was actually thinking this should be closed - never heard back and while I like the idea, the problem I was trying to solve can be solved other ways, this was my first learning experience with node on the server - so will close for now, ping me if there is interest in re-opening

@eriktrom eriktrom closed this Mar 20, 2016
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

Successfully merging this pull request may close these issues.

2 participants