Skip to content

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

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Kingdutch
Copy link
Collaborator

The codegen tool did not yet account for reserved keywords when
generating code. This caused a query parameter such as type to
generate code that would refuse to compile.

To fix this a list of reserved keywords are added that get an underscore
added as suffix.

Fixes #30

The codegen tool did not yet account for reserved keywords when
generating code. This caused a query parameter such as `type` to
generate code that would refuse to compile.

To fix this a list of reserved keywords are added that get an underscore
added as suffix.

Fixes #30
@Kingdutch Kingdutch added the bug Something isn't working label Jul 19, 2022
@Kingdutch Kingdutch requested a review from zth July 19, 2022 13:23
@zth
Copy link
Owner

zth commented Jul 19, 2022

Great! Would it be possible to somehow add either a test, or something to the example app that shows this works both for query params and path params?

Coming to think of it, maybe we should have 2 "example" apps? One actual example, and one fixture-style that's just around to show that the codegen works and compiles. That'll make it easy for us to cram "regression fixtures" into there without it being awkward.

@Kingdutch
Copy link
Collaborator Author

Yeah I was thinking about that but wasn't sure where to put it. I'll see what I can do :)

We want to test some cases for which traditional testing tools are not
entirely suitable. For example testing that a `routes.json` file turns
into compilable ReScript code. For this we create workspaces in
`tests/*` that allow us to test specific cases.

The initial added case demonstrates an issue with using reserved words
in parameter names or query parameter names.
@zth
Copy link
Owner

zth commented Jul 20, 2022

Great addition with the regression test suite! I'll look into this sometime next week, it doesn't feel pressing for the alpha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using a query parameter named type generates invalid Rescript
2 participants