search

linemanjs / lineman

Lineman helps you build fat-client JavaScript apps. It produces happiness by building assets, mocking servers, running specs on every file change
MIT License
1.18k stars 83 forks source link

default `apiProxy.changeOrigin` to true #255

Closed davemo closed 10 years ago

davemo commented 10 years ago

I ran into this while attempting to proxy connections from my localhost lineman instance to an app running on heroku. Without setting apiProxy.changeOrigin to true I just got an unhelpful message from heroku saying 'no such app'.

Spoke with @bjeanes and he suggested this should be the default option as it would be least confusing for people.

Thoughts @jasonkarns @tonyarkles @searls?

https://twitter.com/bjeanes/status/462302173049921536

searls commented 10 years ago

+1 to whatever @bjeanes says. Didn't read. Just merge. 

:cowboyhat:

On Fri, May 2, 2014 at 2:51 PM, David Mosher notifications@github.com wrote:

I ran into this while attempting to proxy connections from my localhost lineman instance to an app running on heroku. Without setting apiProxy.changeOrigin to true I just got an unhelpful message from heroku saying 'no such app'. Spoke with @bjeanes and he suggested this should be the default option as it would be least confusing for people. Thoughts @jasonkarns @tonyarkles @searls? https://twitter.com/bjeanes/status/462302173049921536 You can merge this Pull Request by running: git pull https://github.com/linemanjs/lineman default-apiProxy-changeOrigin-true Or you can view, comment on it, or merge it online at: https://github.com/linemanjs/lineman/pull/255 -- Commit Summary --

davemo commented 10 years ago

Merging, this won't break backwards compatibility either :)

bjeanes commented 10 years ago

lol @searls <3

chringwer commented 9 years ago

This should rather be something like: apiProxyChangeOrigin = !!grunt.config.get("server.apiProxy.changeOrigin")

Otherwise the expression will always evaluate to true, even if someone sets it to false in the config.

davemo commented 9 years ago

We'd be happy to merge a PR @chringwer :)

jasonkarns commented 9 years ago

We probably ought to be using grunt.config.merge so that we can simply provide all defaults and let grunt.config do it's thing.

On Tue, Sep 9, 2014 at 10:01 AM, Christoph Ingwersen < notifications@github.com> wrote:

This should rather be something like: apiProxyChangeOrigin = !!grunt.config.get("server.apiProxy.changeOrigin")

Otherwise the expression will always evaluate to true, even if someone sets it to false in the config.

— Reply to this email directly or view it on GitHub https://github.com/linemanjs/lineman/pull/255#issuecomment-54973003.

searls commented 9 years ago

:+1: to jason's point. are all our options flat? I'm pretty sure grunt.config.merge is shallow

jasonkarns commented 9 years ago

Documentation says that grunt.config.merge "recursively merges..." so I'm assuming it's deep?

Though I would only suggest we do the config merge within the server task, not within lineman itself. We wouldn't want lineman core aware of the task-specific defaults.

And the server task config specifically is not shallow; apiProxy, web being at least two properties that have children.

On Thu, Sep 11, 2014 at 9:41 AM, Justin Searls notifications@github.com wrote:

[image: :+1:] to jason's point. are all our options flat? I'm pretty sure grunt.config.merge is shallow

— Reply to this email directly or view it on GitHub https://github.com/linemanjs/lineman/pull/255#issuecomment-55264863.

searls commented 9 years ago

Oh yeah definitely only within the task. Trust me Lineman already has its own incredibly complex custom merge scheme

On Thu, Sep 11, 2014 at 11:06 AM, Jason Karns notifications@github.com wrote:

Documentation says that grunt.config.merge "recursively merges..." so I'm assuming it's deep? Though I would only suggest we do the config merge within the server task, not within lineman itself. We wouldn't want lineman core aware of the task-specific defaults. And the server task config specifically is not shallow; apiProxy, web being at least two properties that have children. On Thu, Sep 11, 2014 at 9:41 AM, Justin Searls notifications@github.com wrote:

[image: :+1:] to jason's point. are all our options flat? I'm pretty sure grunt.config.merge is shallow

— Reply to this email directly or view it on GitHub https://github.com/linemanjs/lineman/pull/255#issuecomment-55264863.

Reply to this email directly or view it on GitHub: https://github.com/linemanjs/lineman/pull/255#issuecomment-55277406