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

mountPath for middleware is not supporting array or regex #576

Open
piejanssens opened this issue Oct 25, 2021 · 4 comments
Open

mountPath for middleware is not supporting array or regex #576

piejanssens opened this issue Oct 25, 2021 · 4 comments
Labels
enhancement New feature or request

Comments

@piejanssens
Copy link

piejanssens commented Oct 25, 2021

Expected Behavior

Support arrays for mountPath just like express does

Current Behavior

It's not supported

Steps to Reproduce the Issue

  1. add middleware with array definition for mountPath
    - name: ui5-middleware-simpleproxy
      afterMiddleware: ui5-middleware-stringreplacer
      mountPath:
        - '/ci/'
        - '/v2/'
        - '/sf/'

Context

  • UI5 Module Version (output of ui5 --version when using the CLI): 2.13
  • Node.js Version: 14
  • npm Version: 8.1

Log Output / Stack Trace

Configuration server/customMiddleware/1/mountPath must be of type 'string'

/home/pieter/projects/xxx/app/ui5.yaml:17

15:     - name: ui5-middleware-simpleproxy
16:       afterMiddleware: compression
17:       mountPath:
@piejanssens piejanssens added bug Something isn't working needs triage Needs to be investigated and confirmed as a valid issue that is not a duplicate labels Oct 25, 2021
@piejanssens
Copy link
Author

piejanssens commented Oct 25, 2021

I tried a regex, but that doesn't seem to be recongized as regex and fails when the path is converted to a regex:
mountPath: /\/(ci|v2|sf){1}\/.*/

Stack Trace:
SyntaxError: Invalid regular expression: /^\/\\/(?(?:([^\/]+?))|v2|sf){1}\\/\.(.*)\/?(?=\/|$)/: Invalid group
    at new RegExp (<anonymous>)
    at pathtoRegexp (/home/pieter/projects/xxx/app/node_modules/@ui5/cli/node_modules/path-to-regexp/index.js:128:10)

@piejanssens piejanssens changed the title mountPath for middleware is not supporting array type mountPath for middleware is not supporting array or regex Oct 25, 2021
@RandomByte
Copy link
Member

Sounds reasonable. This is only limited by our schema.

@matz3 can you think of any reasons against allowing arrays and regular expressions here as well?

@RandomByte
Copy link
Member

Also see our documentation:

An optional mountPath for which the middleware function is invoked can be provided. It will be passed to the app.use call (see express API reference).

@matz3
Copy link
Member

matz3 commented Oct 27, 2021

@RandomByte an array should be fine. I just wonder about a regular expression. YAML/JSON don't support RegExp, so we would need to differentiate between a RegExp pattern and a string or path pattern.

@RandomByte RandomByte added enhancement New feature or request and removed bug Something isn't working needs triage Needs to be investigated and confirmed as a valid issue that is not a duplicate labels Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants