-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Fix: Allow arbitrary attribute order in triple-slash directives #62821
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?
Fix: Allow arbitrary attribute order in triple-slash directives #62821
Conversation
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
1 similar comment
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
@Bitshifter-9 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
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 parser bug where triple-slash reference directives required attributes to be in a specific order. The title accurately describes the change ("Fix: Allow arbitrary attribute order in triple-slash directives"), but the description is completely incorrect and appears to be from a different pull request about Go/Mattermost database optimization.
Key Changes
- Updated regex patterns for
fullTripleSlashReferencePathRegExandfullTripleSlashReferenceTypeReferenceDirectiveRegExto allow optional attributes before the mainpathortypesattribute - Added a test case demonstrating that
<reference resolution-mode="import" path="./foo.d.ts" />now works correctly - Applied consistent formatting/indentation improvements throughout
src/compiler/utilities.tsfor better code readability
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/cases/compiler/tripleSlashAttributeOrder.ts | New test case verifying triple-slash directive with resolution-mode before path |
| tests/baselines/reference/tripleSlashAttributeOrder.types | Baseline showing correct type resolution through the non-standard attribute order |
| tests/baselines/reference/tripleSlashAttributeOrder.symbols | Baseline showing correct symbol resolution through the non-standard attribute order |
| tests/baselines/reference/tripleSlashAttributeOrder.js | Baseline showing correct JavaScript emission (directive preserved in output) |
| src/compiler/utilities.ts | Core fix: updated regex patterns with (?:[^>]*?\s+)? to match optional attributes before main attribute; also includes extensive formatting improvements for multi-line conditionals |
| // @Filename: /foo.d.ts | ||
| declare const foo: number; | ||
|
|
||
| // @Filename: /index.ts | ||
| /// <reference resolution-mode="import" path="./foo.d.ts" /> | ||
| foo; |
Copilot
AI
Dec 2, 2025
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.
PR Metadata Issue: The PR title mentions "Fix: Allow arbitrary attribute order in triple-slash directives" but the description talks about "createGroupChannel in server/channels/app/channel.go". This description appears to be from a completely different pull request (likely a Go/Mattermost server PR about database optimization). The PR description should be updated to match the actual changes being made to TypeScript's triple-slash directive parsing.
| /// <reference resolution-mode="import" path="./foo.d.ts" /> | ||
| foo; |
Copilot
AI
Dec 2, 2025
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 test only covers the case where resolution-mode appears before path. Consider adding a test case for the traditional order (path before resolution-mode) to ensure backward compatibility and that both orderings work correctly. For example:
/// <reference path="./foo.d.ts" resolution-mode="import" />
Refactored createGroupChannel in server/channels/app/channel.go
to use SaveMultipleMembers for saving channel members, replacing the previous loop of individual SaveMember calls.
The Problem
The previous implementation of
createGroupChannel
iterated through the list of users and called SaveMember for each one. This resulted in N+1 database queries (where N is the number of users in the group), which is inefficient and can lead to performance issues as the group size increases.
The Solution
I replaced the loop with a single call to SaveMultipleMembers, which allows inserting all channel members in a single database operation (or fewer operations depending on the store implementation). This significantly reduces the database overhead.