path-to-regexp is not listed in dependencies

[matthewpflueger]

Describe the bug
On a fresh install of dependencies, followed by a compile then run of the code in issue #345 I ran into this error immediately:

$> node dist/lib/server/http.js
λ - 67501 - 2021-07-18 03:03:36 - websockets [Context] - Registered: "LoggerToken"
λ - 67501 - 2021-07-18 03:03:36 - http [Context] - Registered: "HttpServerClient"
λ - 67501 - 2021-07-18 03:03:36 - http [Context] - Registered: "LoggerToken"
λ - 67501 - 2021-07-18 03:03:36 - http [Context] - Registered: "HttpServerEventStream"
λ - 67501 - 2021-07-18 03:03:36 - http [Context] - Registered: "WebSocketServer"
λ - 67501 - 2021-07-18 03:03:36 - http [Context] - Registered: "HttpRequestBus"
λ - 67501 - 2021-07-18 03:03:36 - http [Router] - Effect mapped: / GET
λ - 67501 - 2021-07-18 03:03:36 - http [Router] - Effect mapped: /(.*) *
(node:67501) UnhandledPromiseRejectionWarning: TypeError: path_to_regexp_1.pathToRegexp is not a function
    at node_modules/@marblejs/websockets/dist/operators/mapToServer/websocket.mapToServer.operator.js:13:39
    at Array.map (<anonymous>)
    at Object.exports.mapToServer (node_modules/@marblejs/websockets/dist/operators/mapToServer/websocket.mapToServer.operator.js:12:35)
    at upgrade$ (dist/lib/server/http.js:19:107)
    at event$ (dist/lib/server/http.js:26:39)
    at node_modules/@marblejs/core/dist/http/server/http.server.js:48:43
    at Generator.next (<anonymous>)
    at fulfilled (node_modules/@marblejs/core/dist/http/server/http.server.js:5:58)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:67501) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:67501) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

To Reproduce
Before running the code in #345 I deleted the node_modules, did a fresh install, and compiled/ran bumping into this error. The package path-to-regexp is not listed in marblejs dependencies anywhere that I saw and I think I ran into a version upgrade of the path-to-regexp that is incompatible...

Expected behavior
For the mapToServer operator to not throw an error...

Desktop (please complete the following information):

• OS: Mac
• Package + Version: rxjs 6.6.2 (this was the cause of the delete and fresh install of dependencies), marble 3.5.0
• Node version: 14

Additional context

[matthewpflueger]

Ugh, never mind, I was looking for the dependency to be declared in the websockets package but I see you've got it declared in core.

[matthewpflueger]

Double ugh, so my situation is that I am using a marblejs in a mono-repo. Some other project has a dependency on path-to-regexp that is really old. Using yarn resolutions I am able to get the right path-to-regexp installed for marblejs BUT because path-to-regexp is not declared as dependency on the websockets package (it is only on the core package) then when using mapToServer it finds the old path-to-regexp. Add a global resolution for path-to-regexp to 6.1.0 fixes it but this isn't the correct fix as the resolution should only be necessary for websockets.

To better explain, here is my resolutions section in package.json:

{
  "resolutions": {
    "@marblejs/core/path-to-regexp": "^6.1.0",
    "@marblejs/websockets/path-to-regexp": "^6.1.0",
    "react-router/path-to-regexp": "^1.7.0",
    "path-to-regexp": "^6.1.0"
  }
}

• The @marblejs/core/path-to-regexp resolution doesn't work because websockets doesn't find it there, it finds the root path-to-regexp
• The @marblejs/websockets/path-to-regexp resolution doesn't work because websockets doesn't declare path-to-regexp as a dependency (it should though)
• react-router is one of the packages using the old path-to-regexp version
• The global resolution fixes my issue because websockets finds the package at the root level

[matthewpflueger]

I added dependency-cruiser to my project and found similar mistakes where I was using a dependency without declaring it in package.json (so easy to do with a monorepo). The default generated rules for dependency-cruiser would have caught that @marblejs/websockets was using path-to-regexp without it being declared...

[JozefFlakus]

@matthewpflueger ... sorry for my late response but I have a lot of other stuff to do besides my Opensource "job". I'll try to prioritize your findings and will back to you with my suggesions and feedback. 🙌

[JozefFlakus]

Here is the problem:
https://github.com/marblejs/marble/blob/master/packages/websockets/src/operators/mapToServer/websocket.mapToServer.operator.ts

As you suggested, the solution is to add path-to-regexp as a dependency. I thought that maybe it could be somehow packed as an internal utility function of @marblejs/core package so that @marblejs/websockets can reuse it, but then the result will be exactly the same - @marblejs/core has to include it... The thing is that with an introduction of version v4.0 there will be an additional module @marblejs/http that will have a direct dependency to path-to-regexp... 🤷

[matthewpflueger]

@matthewpflueger ... sorry for my late response but I have a lot of other stuff to do besides my Opensource "job". I'll try to prioritize your findings and will back to you with my suggesions and feedback. 🙌

No worries and totally get it - appreciate you taking a look!