-
Notifications
You must be signed in to change notification settings - Fork 23
Create a new generated npm workspace for typescript ANTLR generated code #1371
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
base: main
Are you sure you want to change the base?
Create a new generated npm workspace for typescript ANTLR generated code #1371
Conversation
FYI: I was away last week. Will be reviewing this today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution. Before we accept this, I have some suggestions. These suggestions should add this into our current build process to prevent it from going stale before actually using it.
"build": "tsc", | ||
"clean": "rimraf dist" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"build": "tsc", | |
"clean": "rimraf dist" | |
"build": "tsc", | |
"clean": "rm -rf dist" |
We should rely on the system rm command.
"typescript": "^5.0.0", | ||
"rimraf": "^5.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can remove this
"typescript": "^5.0.0", | |
"rimraf": "^5.0.0" | |
"typescript": "^5.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, we should have a common tsconfig that both the frontend and this package extend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a new target like: build-antlr-ts
It should call the npm run build command in the lib/gen/antlrv4/ts-antlrv4
directory.
We currently have one shot commands such as make start-local and make debug-local and make precommit and make playwright-test. It will be necessary to add this target as a pre-requisite target for those.
Context
As described in #716, we're looking for opportunities to do some client side search query validation or autocompletion, so we need to be able to get ANTLR generated code from the typescript side. This PR will unlock #717 and #718
Changes
Makefile
so that it generates TypeScript code tolib/gen/antlrv4/ts-antlrv4
along with Go code generationpackage.json
file to include the new workspace -lib/gen/antlrv4/ts-antlrv4
package.json
andtsconfig.json
to ts-antlrv4 so that it can later be imported into frontend code as well as get compiled..gitignore
to exclude generated ANTLR filesScreenshot with generated typescript code:
With the changes, we should be able to import the package to frontend with:
package.json (frontend):
any ts file (frontend):