Ensure reserved names in params or query don't break ReScript

package.json

@@ -7,7 +7,8 @@
   "private": true,
   "workspaces": [
     "packages/*",
-    "examples/*"
+    "examples/*",
+    "tests/*"
   ],
   "scripts": {
     "build": "yarn build:libs && yarn build:examples",

packages/rescript-relay-router/cli/RescriptRelayRouterCli__Codegen.res

@@ -7,9 +7,41 @@ let wrapInOpt = str => `option<${str}>`
 // "builtins" we use, like "environment". This helps handle that by checking for
 // collisions, letting us refer to safe names where needed.
 module SafeParam = {
-  type t = Actual(string) | CollisionPrevented({realKey: string, collisionProtectedKey: string})
+  type t =
+    | Actual(string)
+    | CollisionPrevented({realKey: string, collisionProtectedKey: string})
+    | RescriptKeyword({realKey: string, collisionProtectedKey: string})
 
   let protectedNames = ["environment", "pathParams", "queryParams", "location"]
+  // As per https://rescript-lang.org/docs/manual/latest/reserved-keywords
+  let reservedKeywords = [
+    "and",
+    "as",
+    "assert",
+    "constraint",
+    "else",
+    "exception",
+    "external",
+    "false",
+    "for",
+    "if",
+    "in",
+    "include",
+    "lazy",
+    "let",
+    "module",
+    "mutable",
+    "of",
+    "open",
+    "rec",
+    "switch",
+    "true",
+    "try",
+    "type",
+    "when",
+    "while",
+    "with",
+  ]
 
   type paramType = Param(string) | QueryParam(string)
 
@@ -18,12 +50,16 @@ module SafeParam = {
     | QueryParam(paramName) =>
       if params->Js.Array2.includes(paramName) || protectedNames->Js.Array2.includes(paramName) {
         CollisionPrevented({realKey: paramName, collisionProtectedKey: "queryParam_" ++ paramName})
+      } else if reservedKeywords->Js.Array2.includes(paramName) {
+        RescriptKeyword({realKey: paramName, collisionProtectedKey: paramName ++ "_"})
       } else {
         Actual(paramName)
       }
     | Param(paramName) =>
       if protectedNames->Js.Array2.includes(paramName) {
         CollisionPrevented({realKey: paramName, collisionProtectedKey: "param_" ++ paramName})
+      } else if reservedKeywords->Js.Array2.includes(paramName) {
+        RescriptKeyword({realKey: paramName, collisionProtectedKey: paramName ++ "_"})
       } else {
         Actual(paramName)
       }
@@ -32,12 +68,14 @@ module SafeParam = {
 
   let getSafeKey = key =>
     switch key {
-    | Actual(key) | CollisionPrevented({collisionProtectedKey: key}) => key
+    | Actual(key)
+    | CollisionPrevented({collisionProtectedKey: key})
+    | RescriptKeyword({collisionProtectedKey: key}) => key
     }
 
   let getOriginalKey = key =>
     switch key {
-    | Actual(key) | CollisionPrevented({realKey: key}) => key
+    | Actual(key) | CollisionPrevented({realKey: key}) | RescriptKeyword({realKey: key}) => key
     }
 }
 

tests/README.md

@@ -0,0 +1,6 @@
+# Tests
+
+Packages in this folder are not intended to be full fledged examples but instead serve as bare bones regression and integration tests.
+
+**Test cases MUST NOT include a `build` script to ensure that tooling can be rebuild in the repository even if tests are failing**
+**Test cases MUST include a `test` script that returns a non-zero status code on test failure**

tests/router-generation/.gitignore

@@ -0,0 +1,20 @@
+# Ignore common system files
+.DS_Store
+
+# Ignore node modules
+node_modules
+
+# Ignore rescript folders
+lib
+.bsb.lock
+.merlin
+
+# Ignore build output
+*.mjs
+
+# Ignore generated files but allow keeping the folder itself.
+src/__generated__/**
+
+# In this project we also ignore the generated route files themselves because
+# they're not part of the test set-up.
+src/routes/__generated__

tests/router-generation/README.md

@@ -0,0 +1,7 @@
+# Test - Router Generation
+
+This test case ensures that the R3 CLI outputs ReScript code that is accepted by the ReScript compiler based on a test `routes.json` file.
+
+## Reading test failures
+
+In case ReScript fails to build it will provide an error for a specific filename. The filename can be used to deduce the specific test case in `routes.json` that failed.

tests/router-generation/bsconfig.json

@@ -0,0 +1,30 @@
+{
+  "name": "@rescript-relay-router-test/router-generation",
+  "reason": {
+    "react-jsx": 3
+  },
+  "version": "0.0.0",
+  "sources": [
+    {
+      "dir": "src",
+      "subdirs": true
+    }
+
+  ],
+  "package-specs": [
+    {
+      "module": "es6",
+      "in-source": true
+    }
+  ],
+  "suffix": ".mjs",
+  "ppx-flags": [],
+  "bs-dependencies": [
+    "@rescript/react",
+    "rescript-relay",
+    "rescript-relay-router"
+  ],
+  "pinned-dependencies": [
+    "rescript-relay-router"
+  ]
+}

tests/router-generation/package.json

@@ -0,0 +1,29 @@
+{
+  "name": "@rescript-relay-router-test/router-generation",
+  "description": "Test that ReScript code generated by the router compiles properly.",
+  "version": "0.0.0",
+  "private": true,
+  "engines": {
+    "node": ">=16"
+  },
+  "type": "module",
+  "packageManager": "[email protected]",
+  "scripts": {
+    "test": "yarn run test:generate && yarn run test:build",
+    "test:generate": "rescript-relay-router generate",
+    "test:build": "rescript build -with-deps"
+  },
+  "dependencies": {
+    "@rescript/react": "0.10.3",
+    "history": "^5.2.0",
+    "react": "^18.2.0",
+    "react-dom": "^18.2.0",
+    "react-relay": "13.2.0",
+    "react-router": "^6.2.1",
+    "relay-runtime": "13.2.0",
+    "rescript": "9.1.4",
+    "rescript-relay": "1.0.0-beta.23",
+    "rescript-relay-router": "workspace:^",
+    "vite": "2.9.9"
+  }
+}

tests/router-generation/rescriptRelayRouter.config.cjs

@@ -0,0 +1,4 @@
+module.exports = {
+  generatedPath: "./src/routes/__generated__",
+  routesFolderPath: "./src/routes",
+};

tests/router-generation/src/routes/routes.json

@@ -0,0 +1,56 @@
+[
+  // Test the simple base case.
+  {
+    "path": "/",
+    "name": "RootTest",
+    "children": []
+  },
+  // Test that children work as expected.
+  {
+    "path": "/withChildrenTest",
+    "name": "WithChildrenTest",
+    "children": [
+      {
+        "path": "childRoute",
+        "name": "ChildRoute"
+      }
+    ]
+  },
+  // Test that reserved names in query parameters are escaped.
+  {
+    "path": "/queryParamTest",
+    "name": "QueryParamTest",
+    "children": [
+      {
+        "path": "childRoute?type=string&of=string&with=string",
+        "name": "TestCase"
+      }
+    ]
+  },
+  // Test that reserved names in route parameters are escaped.
+  {
+    "path": "/routeParamTest",
+    "name": "RouteParamTest",
+    "children": [
+      {
+        "path": ":type/:of/:with",
+        "name": "TestCase"
+      }
+    ]
+  },
+  // Test that conflicts between route and query parameters are resolved
+  {
+    "path": "/paramConlictTest",
+    "name": "ParamConflictTest",
+    "children": [
+      {
+        "path": ":safeParam?safeParam=int",
+        "name": "SafeParamTestCase"
+      },
+      {
+        "path": ":type?type=int",
+        "name": "ReservedParamTestCase"
+      }
+    ]
+  }
+]

yarn.lock

@@ -779,6 +779,24 @@ __metadata:
   languageName: unknown
   linkType: soft
 
+"@rescript-relay-router-test/router-generation@workspace:tests/router-generation":
+  version: 0.0.0-use.local
+  resolution: "@rescript-relay-router-test/router-generation@workspace:tests/router-generation"
+  dependencies:
+    "@rescript/react": 0.10.3
+    history: ^5.2.0
+    react: ^18.2.0
+    react-dom: ^18.2.0
+    react-relay: 13.2.0
+    react-router: ^6.2.1
+    relay-runtime: 13.2.0
+    rescript: 9.1.4
+    rescript-relay: 1.0.0-beta.23
+    rescript-relay-router: "workspace:^"
+    vite: 2.9.9
+  languageName: unknown
+  linkType: soft
+
 "@rescript-relay-router/vitest@workspace:^, @rescript-relay-router/vitest@workspace:packages/rescript-relay-router-vitest":
   version: 0.0.0-use.local
   resolution: "@rescript-relay-router/vitest@workspace:packages/rescript-relay-router-vitest"