-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Remove cancellationToken.js #60250
Remove cancellationToken.js #60250
Conversation
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.
No reason to have it separate that I know of.
@typescript-bot perf test startup-only |
@jakebailey Here they are:
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
let cancellationPipeName: string | undefined; | ||
for (let i = 0; i < args.length - 1; i++) { | ||
if (args[i] === "--cancellationPipeName") { | ||
cancellationPipeName = args[i + 1]; | ||
break; | ||
} | ||
} |
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.
@jakebailey Perhaps ts.server.findArgument()
can be used here as well?
TypeScript/src/tsserver/nodeServer.ts
Line 94 in 30979c2
const mode = ts.server.findArgument("--serverMode"); |
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.
This PR was a pure code move; that could be a refactor later, sure
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.
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.
That was quick! Thanks.
cancellationToken.js
is a file which contains an implementation of a file-based cancellation token. It is loaded dynamically bytsserver.js
, but nowhere else. As far as I can tell, there's no reason to not just include its code intsserver.js
directly and drop the extra file andrequire
.(Going through the repo history also doesn't indicate why this was a separate file; perhaps there's a good reason for this to exist and I lack context.)