Skip to content

compiler: resolve paths in middleware params#74

Merged
bajtos merged 1 commit into
masterfrom
feature/relative-paths-in-middleware-params
Dec 1, 2014
Merged

compiler: resolve paths in middleware params#74
bajtos merged 1 commit into
masterfrom
feature/relative-paths-in-middleware-params

Conversation

@bajtos

@bajtos bajtos commented Nov 28, 2014

Copy link
Copy Markdown
Member

Introduce a convention for specifying relative paths in middleware params: values prefixed with $! and starting with ./ or ../ are resolved relatively to middleware.json.

Example (server/middleware.json)

{
  "files": {
    "loopback#static": {
      "params": "$!../client"
      // resolved to `client`
    },
    "loopback#static": {
      "params": "$!./public"
      // resolved to `server/public`
    }
  }
}

/to @ritch @raymondfeng please review, especially whether the prefix value makes sense to you too. It was inspired by hashbang (#!) used by client-side MVC frameworks.

I have discovered this use case while reworking the template scaffolded by slc loopback as part of strongloop/loopback-workspace#167

Introduce a convention for specifying relative paths in middleware
params: values prefixed with `$!` and starting with `./` or `../`
are resolved relatively to `middleware.json`.

Example:

    {
      "files": {
        "loopback#static": {
          "params": "$!../client"
        },
        "loopback#static": {
          "params": "$!./public"
        }
      }
    }
@bajtos bajtos added the #review label Nov 28, 2014
@bajtos bajtos added this to the #Epic: Components milestone Nov 28, 2014
@ritch

ritch commented Dec 1, 2014

Copy link
Copy Markdown
Member

Maybe it requires a name eg. $APP_ROOT... Although I don't mind the current proposal. Feel free to move forward with it if you disagree with using a descriptive name.

@ritch ritch assigned bajtos and unassigned ritch Dec 1, 2014
@bajtos

bajtos commented Dec 1, 2014

Copy link
Copy Markdown
Member Author

Maybe it requires a name eg. $APP_ROOT... Although I don't mind the current proposal. Feel free to move forward with it if you disagree with using a descriptive name.

I expect this feature will be used quite frequently, therefore I prefer to keep the prefix as short as possible.

Also note that the path is relative to the middleware.json file, not to the project root. I.e. in a typical loopback project scaffolded by slc loopback, you have server/middleware.json and paths are relative to server, not to the project root. It seems to me that APP_ROOT can create confusion as many developers may assume that APP_ROOT is the project root.

@raymondfeng

Copy link
Copy Markdown
Member

I'm leaning toward to use typical variable delimiters for JS templates. For example,

<%= __dirname %>
{{ __dirname }}

It will allow us to use existing template engines to parse the templates and inject values.

I'm fine to have a shortcut for this special case with a prefix such as '#!' (why not use #!?).

@bajtos

bajtos commented Dec 1, 2014

Copy link
Copy Markdown
Member Author

I'm fine to have a shortcut for this special case with a prefix such as '#!' (why not use #!?).

  • $ denotes a variable/computed value in languages like Bash/Shell, PHP, Perl
  • hashbang paths are usually absolute (e.g. #!/home)
  • front-end developers may get confused about #! having a different meaning on the server side

@bajtos

bajtos commented Dec 1, 2014

Copy link
Copy Markdown
Member Author

I'm leaning toward to use typical variable delimiters for JS templates.

That may be a good solution for #75 👍

@raymondfeng

Copy link
Copy Markdown
Member

I'm fine to use $! as the shortcut for the __dirname of middleware.json.

bajtos added a commit that referenced this pull request Dec 1, 2014
…leware-params

compiler: resolve paths in middleware params
@bajtos
bajtos merged commit 3dfa3bc into master Dec 1, 2014
@bajtos bajtos removed the #review label Dec 1, 2014
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.

3 participants