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

Start the FastBoot server immediately when --build=false #257

Closed

Conversation

treywood
Copy link

@treywood treywood commented Sep 6, 2016

Going through the User Guide, I noticed that using the suggested command for debugging:

$ ember build && node --debug-brk ./node_modules/.bin/ember fastboot --no-build

would never start the server. After poking around I found that $ ember fastboot --no-build would start and immediately exit since no 'postBuild' event is fired to start the server. The changes in this PR cause the server to start immediately when --build=false

@tomdale
Copy link
Contributor

tomdale commented Dec 16, 2016

@treywood Hey, thanks for the PR!

We discussed this PR on the call today. I am a little leery about accepting this patch as-is. There is some work on-going to give addons the ability to patch into the built-in Ember Express server. That seems like a better option than hard-coding in special case behavior about the build into the server. (IMO the server already shouldn't have any knowledge of the build system, and I don't want to canonize that bad architectural system by building on it further.)

That said, I think I would be OK accepting a PR in the interim if there was a different event than postBuild that we could listen for that was triggered whether or not the build was kicked off—although I'm not sure if such a thing exists. But I'm not comfortable with the server starting to parse build-specific CLI options.

@simonihmig
Copy link
Contributor

Just found this by coincidence. Submitted a PR for that issue recently that was already merged: #346. Should be able to close this then!

@treywood treywood closed this Feb 6, 2017
@rwjblue
Copy link
Member

rwjblue commented Feb 6, 2017

Dang, I'm sorry about missing this PR originally @treywood.

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.

4 participants