lwsjs / local-web-server

A lean, modular web server for rapid full-stack development.
MIT License
1.21k stars 85 forks source link

Merge rewrite rules from multiple .local-web-server.json files #53

Closed djKianoosh closed 7 years ago

djKianoosh commented 7 years ago

If we have two .local-web-server.json files, say in the current directory and in the user's $HOME, configurations like port get merged properly, but if both files have rewrite rules, then only the rules in the current directory show up with ws --config.

Maybe it needs a deep merge?

75lb commented 7 years ago

hi @djKianoosh .. yes, a deep merge would fix this issue in your case but this behaviour would sometimes be undesirable..

typically, a user puts their default config in $HOME.. later, if they decide to add some config within the project folder they typically want this config to override the $HOME config, not extend it..

leaving ticket open for further suggestions.. ideally, we could choose whether project config merges with or overrides $HOME config.

djKianoosh commented 7 years ago

Yeah, maybe a separate config that controls whether to override completely or merge would help there.

For our use case, which is sort of a custom microservice environment, we could have several projects that would like to extend a base config (port, and a base rewrite rule to our dev server). Then, depending on what project a developer is working on, that project would override the the rewrite rule to hit a local api server for that particular api they are working on. Make sense?

djKianoosh commented 7 years ago

@75lb let me know how I can help, if you need a PR or example configs. I tried to find where in the code to make this change but got lost in the weeds a little bit. If you can point me in the right direction I may be able to help.

djKianoosh commented 7 years ago

Mainly looking at https://github.com/75lb/local-web-server/blob/master/bin/cli.js#L133 Where you're using Object.assign http://www.2ality.com/2014/01/object-assign.html

The question is what deep merge function would you prefer to use? Lodash's or a custom one like in http://stackoverflow.com/questions/27936772/deep-object-merging-in-es6-es7

And then what do you want to call the option to bypass the normal extend vs doing the deep merge?

djKianoosh commented 7 years ago

I think I see the loading up of configs is actually done in your config-master lib: https://github.com/75lb/config-master/blob/master/lib/config-master.js#L29

djKianoosh commented 7 years ago

I made a pr in that repo to add a deep merge option: https://github.com/75lb/config-master/pull/2

75lb commented 7 years ago

hi, thanks for the PR - i will take a look tonight after work and Parent's Evening 👍

75lb commented 7 years ago

i have released this feature on a tag as it needs testing.. please install and let me know your thoughts:

$ npm install -g local-web-server@deep

then run it with this flag:

$ ws --deep-merge-config

see the config master deep branch to see the changes.

djKianoosh commented 7 years ago

So I did a super quick dirty example to see it in action with that flag. It's here: https://github.com/djKianoosh/local-web-server-deep-merge-example

When I run it inside the subdir I get the following rules:

  "rewrite": [
    {
      "from": "one",
      "to": "two"
    },
    {
      "from": "two",
      "to": "three"
    }

Are those valid rewrite rules? Shouldn't it be something like:

  "rewrite": [
    {
      "from": "two",
      "to": "three"
    }

For reference, the .local-web-server.json in the parent dir looks like:

{
  "port": 8888,
  "rewrite": [
     { "from": "one", "to": "two" }
  ]
}

and the one in the subdir looks like:

{
  "rewrite": [
    { "from": "two", "to": "three" }
  ]
}
djKianoosh commented 7 years ago

I see what you mean. You have it right. I had to look again at the example you provided in https://runkit.com/75lb/58066f28868d5c0014804c73

75lb commented 7 years ago

so is it working correctly now in your real life case?

djKianoosh commented 7 years ago

Actually the order of precedence matters. So we want the rewrite rules in the local directory to be more important/specific than the ones in the parent directories. I'm thinking you can either reverse the order that you're doing them now, or add some numeric precedence in the rewrite rule object to sort by (more complicated, granted).

75lb commented 7 years ago

i avoid deep merging in all my tools (it's an opinionated area with different strategies to suit different scenarios) but in the specific case of rewrite rules, i think the behaviour you described would be the appropriate default..

i will look into reversing the order of precedence tonight, to make your local settings higher priority..

djKianoosh commented 7 years ago

thanks @75lb you're the man

75lb commented 7 years ago

this is resolved in v2 which is in progress and available for preview. Config files are now plain javascipt, giving you freedom to share and merge options however you like.

The config file (lws.config.js by default) should look something like this:

module.exports = {
  rewrite: [
    {
      from: '/resources/*',
      to: 'http://remote-api.org:8080/resources/$1'
    }
  ],
  directory: 'src',
  'log.format': 'none'
}

An example of how you might share and merge options:

/* pull options from the env */
const remoteAPI = process.env.REMOTE_API

/* .. or an installed package */
const sharedOptions = require('shared-options')

/* merge them to taste */
const options = Object.assign({}, sharedOptions, {
  rewrite: [
    {
      from: '/resources/*',
      to: `http://${remoteAPI}/resources/$1`
    }
  ]
})

module.exports = options
75lb commented 7 years ago

fixed and released in v2.0.0

djKianoosh commented 7 years ago

Thanks and congrats on the release!

Here's another way to do what I was trying to do, which is have a default config in user home, with local rewrite rules (in a given project for example) taking precedence:

In ~/project/lws.config.js :

const options = require(require('os').homedir() + '/lws.config.js');

options.rewrite.unshift({
    from: '/base/higher/precedence/*',
    to: 'http://localhost:8080/base/higher/precedence/$1'
});

module.exports = options;

Using .unshift() just to put the item to the front of the array as opposed to .concat().

And in ~/lws.config.js:

module.exports = {
  port: 80,
  rewrite: [
    {
      from: "/base/*",
      to: "https://internal/base/$1"
    }
  ]
}

Results in the following with version 2.0.1:

$ ws --config
{ port: 80,
  stack:
   Stack [
     BodyParser,
     RequestMonitor,
     Log,
     Cors,
     Json,
     Rewrite,
     Blacklist,
     ConditionalGet,
     Mime,
     Compress,
     MockResponse,
     SPA,
     Static,
     Index ],
  rewrite:
   [ { from: '/base/higher/precedence/*',
       to: 'http://localhost:8080/base/higher/precedence/$1' },
     { from: '/base/*', to: 'https://internal/base/$1' } ],
  config: true }
75lb commented 7 years ago

looks good, thanks for confirming you got it working! i will write a tutorial on how to share rewrite rules at some point 👍