Fix: Allow arbitrary attribute order in triple-slash directives#62821
Fix: Allow arbitrary attribute order in triple-slash directives#62821Bitshifter-9 wants to merge 1 commit intomicrosoft:mainfrom
Conversation
typescript-bot
added
the
For Uncommitted Bug
|
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. |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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" />
|
With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies). Next steps for PRs:
|
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.