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

allow for ./server to export express app #3579

Merged
merged 1 commit into from
Mar 23, 2015

Conversation

calvinmetcalf
Copy link
Contributor

the whole app factory pattern is a bit foreign for express apps as there is a build in mechanism for adding routes from one express app to another, that is the .use method. A common pattern is for the server function to export an app that can then be passed as middleware to somewhere else or get used with http[s].createServer, so this pull adds compatablity for having a server/index that does something like

var app = module.exports = require('express')();

by checking if the function exported by the server module has a length of 3, i.e. is of the form

var app = function(req, res, next) {
  app.handle(req, res, next);
};

which is what an express app is

@stefanpenner
Copy link
Contributor

Seems reasonable. This is just for dev harness stuff right? You aren't actually building your backend in the context of a cli app?

@calvinmetcalf
Copy link
Contributor Author

yeah the front end guy wants to be able to go ember serve and get live reload during dev but the backend guy doesn't want to change a thing in his code

@stefanpenner
Copy link
Contributor

yeah the front end guy wants to be able to go ember serve and get live reload during dev but the backend guy doesn't want to change a thing in his code

why not just use the proxy? HTTP is a wonderful interface. And i don't want to debug peoples backend code that leaked into the front-end project. :P

@calvinmetcalf
Copy link
Contributor Author

so in the example there is no separate front end and back end projects. the express app and the ember app live together in the same project (and get deployed together to beanstalk) but ember-cli is only used for the ember half of it with express handling both the api and the serving of static files (though doesn't have to). cc @knownasilya

@knownasilya
Copy link
Contributor

why not just use the proxy?

Because if you have custom flags or builds, the proxy doesn't work because it tries to start the server and doesn't know about those extra things.

@knownasilya
Copy link
Contributor

Here's an error:

ember serve -pxy http://localhost:7012
The package `ember-data` is not a properly formatted package, we have used a fallback lookup to resolve it at `/Users/me/myproject/node_modules/ember-data`. This is generally caused by an addon not having a `main` entry point (or `index.js`).
version: 0.2.0
The package `ember-data` is not a properly formatted package, we have used a fallback lookup to resolve it at `/Usersme/myproject/node_modules/ember-data`. This is generally caused by an addon not having a `main` entry point (or `index.js`).
/Users/me/myproject/server/index.js:52
app.post('/', (req, res, next) => {
                               ^^
Unexpected token =>
/Users/me/myproject/server/index.js:52
app.post('/', (req, res, next) => {
                               ^^
SyntaxError: Unexpected token =>
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:427:25)
    at Object.Module._extensions..js (module.js:462:10)
    at Module.load (module.js:339:32)
    at Function.Module._load (module.js:294:12)
    at Module.require (module.js:349:17)
    at require (module.js:368:17)
    at Project.require (/Users/me/myproject/node_modules/ember-cli/lib/models/project.js:216:12)
    at Class.module.exports.Task.extend.processAppMiddlewares (/Users/me/myproject/node_modules/ember-cli/lib/tasks/server/express-server.js:78:33)
    at Class.<anonymous> (/Users/me/myproject/node_modules/ember-cli/lib/tasks/server/express-server.js:171:21)

Normally the server is started with node --harmony_arrow_functions index via npm start.

@knownasilya
Copy link
Contributor

Ping @stefanpenner

@stefanpenner
Copy link
Contributor

So I don't dis-agree with the above API improvement, but I do not feel good about your use-case.

I will very likely merge this, but i want to understand what is going on here.

I am extremely confused why ember-cli can't just proxy to the other node app, or the other node-app proxies back to ember-cli. HTTP is the boundary your app uses in project between the client-side and server-side, why can't it be that in development?

@stefanpenner
Copy link
Contributor

why not just use the proxy?
Because if you have custom flags or builds, the proxy doesn't work because it tries to start the server and doesn't know about those extra things.

why not just use those flags when starting ember cli?

@stefanpenner
Copy link
Contributor

also re-starting app-veyor

@knownasilya
Copy link
Contributor

why not just use those flags when starting ember cli?

That is an alternative as well.

node --harmony_arrow_functions `which ember` serve -pxy http://localhost:7012

but with the proxy you lose out on the server/client live reload, which is the main reason for this change, so that you can have those with a custom express app.

@stefanpenner
Copy link
Contributor

but with the proxy you lose out on the server/client live reload, which is the main reason for this change, so that you can have those with a custom express app.

are you aware that ember-cli has a proxy built-in, that works with live-reload?

@knownasilya
Copy link
Contributor

@stefanpenner was not aware.

This doesn't work, so this PR is necessary it seams:

node --harmony_arrow_functions `which ember` serve -pxy http://localhost:7012
The package `ember-data` is not a properly formatted package, we have used a fallback lookup to resolve it at `/Users/me/myproject/node_modules/ember-data`. This is generally caused by an addon not having a `main` entry point (or `index.js`).
version: 0.2.0
The package `ember-data` is not a properly formatted package, we have used a fallback lookup to resolve it at `/Users/me/myproject/node_modules/ember-data`. This is generally caused by an addon not having a `main` entry point (or `index.js`).
ember-cli expected ./server/index.js to be the entry for your mock or proxy server
TypeError: ember-cli expected ./server/index.js to be the entry for your mock or proxy server
    at Class.module.exports.Task.extend.processAppMiddlewares (/Users/me/myproject/node_modules/ember-cli/lib/tasks/server/express-server.js:80:15)
    at Class.<anonymous> (/Users/me/myproject/node_modules/ember-cli/lib/tasks/server/express-server.js:171:21)
    at lib$rsvp$$internal$$tryCatch (/Users/me/myproject/node_modules/ember-cli/node_modules/rsvp/dist/rsvp.js:489:16)
    at lib$rsvp$$internal$$invokeCallback (/Users/me/myproject/node_modules/ember-cli/node_modules/rsvp/dist/rsvp.js:501:17)
    at /Users/me/myproject/node_modules/ember-cli/node_modules/rsvp/dist/rsvp.js:1095:13
    at lib$rsvp$asap$$flush (/Users/me/myproject/node_modules/ember-cli/node_modules/rsvp/dist/rsvp.js:1290:9)
    at process._tickCallback (node.js:349:13)

@calvinmetcalf
Copy link
Contributor Author

@stefanpenner I think we are conflating 2 issues here,

  • one issue is that our team and ember have conflicting conventions that are causing a problem with ember-cli looking in the server directory and running it even if you are only want to use the proxy, adding the ability to have server not try to run anything when using the proxy (i.e. --just-proxy or something) would solve that
  • the other issue is what this pull is trying to solve, i.e. that there is a more idiomatic way to add routes to an express server

@stefanpenner
Copy link
Contributor

the other issue is what this pull is trying to solve, i.e. that there is a more idiomatic way to add routes to an express server

this I am +1 on

one issue is that our team and ember have conflicting conventions that are causing a problem with ember-cli looking in the server directory and running it even if you are only want to use the proxy, adding the ability to have server not try to run anything when using the proxy (i.e. --just-proxy or something) would solve that

This I would prefer to discourage as ember-cli aims to be a UI development harness, as such the exposed express server really aims to satisfy the following needs:

  1. complex proxy'ing use-cases
  2. fake server until the backend is ready

Although one can build a full-backend into this, it really isn't supported. I would suggest instead utilizing the proxy and HTTP as a boundary between client and server. This prevents us from conflating package.json with client-side + build tool + server side dependencies. It also does the same for the express middleware stack.

@knownasilya
Copy link
Contributor

I think we're all on the same page now, and it looks like we uncovered an issue with the proxy feature, which should never try to initialize server/index.js, but it does.

@stefanpenner
Copy link
Contributor

@stefanpenner was not aware.

#1530 was added by the groupon folk for this use-case, there are also now generates for this.

--proxy also exists, for quick short-hand usages.

@stefanpenner
Copy link
Contributor

I think we're all on the same page now, and it looks like we uncovered an issue with the proxy feature, which should never try to initialize server/index.js, but it does.

instead it should do as @calvinmetcalf changed it do via arity check, correct?

@knownasilya
Copy link
Contributor

@stefanpenner that might solve it, but in reality ember serve --proxy shouldn't care what's in the server/ directory.

@stefanpenner
Copy link
Contributor

@stefanpenner that might solve it, but in reality ember serve --proxy shouldn't care what's in the server/ directory.

it does care, because this is were it gets its extended proxy information from. This is a folder ember-cli expects by convention to produce something it understands.

@stefanpenner
Copy link
Contributor

windows CI issues appear unrelated...

@knownasilya
Copy link
Contributor

@stefanpenner seems like there should be a way to change the server root directory that ember-cli expects, configurable in the .ember-cli and via arguments.

@stefanpenner
Copy link
Contributor

@stefanpenner seems like there should be a way to change the server root directory that ember-cli expects, configurable in the .ember-cli and via arguments.

I don't think so, this directory structure is managed by ember-cli. If you add to it, you are invading it's territory and doing so at your own risk.

This is a config vs convention trade-off I am very interesting in keeping around. Additionally, opening this to configuration is encouraging behavior I do not feel comfortable with.

@knownasilya
Copy link
Contributor

@stefanpenner so you are forcing people to write custom post processing to make their apps work, as well as nesting folders with package.json, and complicating setups that have to work around the ember-cli structure.

server/ is a bad name that is used by other people, name it something else, like mock-server/, fixture-server/, prototype-server/, etc..

Not everyone deploys their client-side code separately, since it complicates infrastructure, deployments and the costs increase, especially if all you do is client work.

@stefanpenner
Copy link
Contributor

Not everyone deploys their client-side code separately, since it complicates infrastructure and deployments, especially if all you do is client work.

This is unfortunate, as ember-cli code is not something we currently feel comfortable deploying to production.

I also do not believe this adds complexity, ember-cli produces static assets that any web-server can host.

ember build --production
scp -R dist/  deployment/path/or/server

@stefanpenner
Copy link
Contributor

server/ is a bad name that is used by other people, name it something else, like mock-server/, fixture-server/, prototype-server/, etc..

I am fine actually with calling it prototype-server or mock-server as i do agree it is a poor name, but not for the collision that brought you to this conversation.

@knownasilya
Copy link
Contributor

@stefanpenner but ember-cli code isn't deployed as working, we do a build like you said, and we serve the built assets from our server in server/.

We do not run ember serve in production.

@stefanpenner
Copy link
Contributor

the reason for my negative feelings toward this, is I have repeatedly been approached with absolutely crazy scenarios node developers have put themselves in. These scenarios break ember-cli features and make development + upgrading incredibly hard.

I also agree, the simplicity of a single project is nice. The solution for this, is put ember-cli managed directory into sub-folder of your server. In theory we could allow mounting ember-cli into some existing middelware stack or similar.

This pain almost always manifests as pain towards ember-cli, when in-reality it is because they have created a crazy monolith.

@knownasilya
Copy link
Contributor

This is not that case. This is the case of using conventions over configuration.

I'll put together another issue, because I totally hijacked this thread 😸

@stefanpenner
Copy link
Contributor

also, please note I will merge this PR, once tests are actually green. Currently appveyor is trolling..

@stefanpenner
Copy link
Contributor

appveyor is failing for unrelated timeouts ...

stefanpenner added a commit that referenced this pull request Mar 23, 2015
allow for ./server to export express app
@stefanpenner stefanpenner merged commit 85763ff into ember-cli:master Mar 23, 2015
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.

None yet

3 participants