-
Notifications
You must be signed in to change notification settings - Fork 54
test: fix slowdown of JS SDK unit tests #2475
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
29a0eb7
test: fix slowdown of JS SDK unit tests
shumkov 8d8d8ae
build: mocha tsc config to build test flles
shumkov b971fce
chore: remove debug output
shumkov 6834130
docs: remove unnecessary comments
shumkov eb3dd9b
build: include test files into test build
shumkov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,6 @@ describe('SDK', function suite() { | |
| }, | ||
| }, | ||
| }; | ||
|
|
||
| clientInstance = new Dash.Client(clientOpts); | ||
| }); | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
🛠️ Refactor suggestion
JSON Formatting: Remove Trailing Comma in
compilerOptions.The additions of
"skipLibCheck","esModuleInterop", and"outDir"are beneficial for optimizing type-checking and module interoperability, and for directing output during compilation. However, please note that valid JSON does not allow a trailing comma in an object. The comma after"outDir": "./build"(line 7) should be removed to avoid JSON parsing errors.📝 Committable suggestion
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.
I think it's a JSON 5 because
tscparses it successfully.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.
🧩 Analysis chain
@shumkov You're absolutely right! TypeScript's configuration files (tsconfig.json) support JSON5 format, which allows trailing commas, comments, and other convenient features. The current syntax is perfectly valid.
🌐 Web query:
💡 Result:
TypeScript's
tsconfig.jsonfiles do not natively support full JSON5 syntax, but they do support a subset of JSON5 features through JSON with Comments (JSONC)[1][5][8]. Here's a breakdown:Key Details
Supported Syntax:
//and/* */)[2][8]Limitations:
tsconfig.jsonas JSONC (JSON with Comments), not full JSON5[5]Implementation Notes
The TypeScript compiler (
tsc) uses a parser that allows:{ // This comment works "compilerOptions": { "target": "ES2022", // Trailing comma allowed }, // Another trailing comma }[2][3][8]
Attempting to use unsupported JSON5 features like unquoted keys:
{ unquotedKey: "value" // Syntax error }will fail compilation[4][9].
Workarounds
For full JSON5 support in configs:
json5to parse.json5files[1][4]Why This Matters
While
tsconfig.jsonallows limited JSON5-like syntax for readability[2][8], developers needing advanced features must use external tooling or alternative config formats[1][4]. The distinction between JSONC and JSON5 is subtle but impacts compatibility[4][9].Citations:
@shumkov I need to correct my previous statement. TypeScript actually uses JSONC (JSON with Comments) format for tsconfig files, not full JSON5. JSONC supports a subset of JSON5-like features, specifically comments and trailing commas, which explains why the trailing comma works in your configuration. The current syntax is indeed valid with JSONC.
✏️ Learnings added