-
Notifications
You must be signed in to change notification settings - Fork 10
Fix syntax error in TypeScript when shape filenames contain hyphens #122
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?
Conversation
…ript identifiers Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
|
@copilot please use https://www.npmjs.com/package/to-valid-identifier rather than defining a custom |
…pm package Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
Done in 99b55f9. Replaced the custom |
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.
Pull request overview
This PR fixes a TypeScript syntax error that occurs when SHACL/ShEx shape filenames contain hyphens (e.g., volunteer-profile-shapes). The fix replaces a custom identifier sanitization approach with the to-valid-identifier npm package, which properly converts filenames to valid TypeScript identifiers.
Key changes:
- Introduced
to-valid-identifierpackage to convert filenames with special characters into valid TypeScript identifiers - Updated all EJS templates to distinguish between
identifierName(for TypeScript code) andfileName(for file paths and comments) - Modified
build.tsto generate and passidentifierNameto templates alongside the originalfileName
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/package.json | Added to-valid-identifier dependency (v1.0.0) |
| package-lock.json | Added lock entries for to-valid-identifier and its dependencies (@sindresorhus/base62 and reserved-identifiers) |
| packages/cli/src/build.ts | Imported to-valid-identifier and added logic to convert fileName to identifierName before passing to templates |
| packages/cli/src/templates/context.ejs | Updated to use identifierName for the exported constant name while keeping fileName for comments |
| packages/cli/src/templates/schema.ejs | Updated to use identifierName for the exported constant name while keeping fileName for comments |
| packages/cli/src/templates/shapeTypes.ejs | Updated to use identifierName for import and variable references while keeping fileName for file paths |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "prompts": "^2.4.2", | ||
| "rdf-dereference-store": "^1.4.0", | ||
| "rdf-namespaces": "^1.13.1", | ||
| "to-valid-identifier": "^1.0.0", |
Copilot
AI
Jan 3, 2026
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.
The to-valid-identifier package requires Node.js >=20, but this package.json doesn't specify an engines field to enforce this requirement. Consider adding an engines field to prevent installation issues on older Node.js versions. For example:
"engines": {
"node": ">=20"
}This would make the requirement explicit and prevent users from encountering runtime errors when using Node.js versions below 20.
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.
@copilot apply changes based on this feedback
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.
Node that Node 18 is now EOL'd (https://nodejs.org/en/about/previous-releases)
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.
Added engines field with "node": ">=20" in d97cb64.
Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
volunteer-profile-shapes)to-valid-identifiernpm package instead of custom functionbuild.tsto import and usetoValidIdentifierfrom the packagetoValidTypescriptIdentifierutility function and testscontext.ejs,schema.ejs,shapeTypes.ejs) to useidentifierNamefor TypeScript code and keepfileNamefor file pathsenginesfield to package.json to specify Node.js >=20 requirement (required byto-valid-identifier)Original prompt
This section details on the original issue you should resolve
<issue_title>Syntax error in typescript with
-</issue_title><issue_description>
Error Message:
Shape: