Skip to content

fix(issues): Use map for project member index#116996

Merged
scttcper merged 4 commits into
masterfrom
scttcper/member-map-index
Jun 5, 2026
Merged

fix(issues): Use map for project member index#116996
scttcper merged 4 commits into
masterfrom
scttcper/member-map-index

Conversation

@scttcper

@scttcper scttcper commented Jun 5, 2026

Copy link
Copy Markdown
Member

Project slugs can collide with Object prototype keys like constructor, which made the assignee selector receive a non-array member list in issue rows.

Use a Map for the project member index so slug lookups are data lookups instead of object property access. Adds coverage for the issue-list path that was crashing.

could use Object.create(null) but switching entirely to a map avoids the problem a little better for the future.

fixes JAVASCRIPT-39FS

Project slugs can collide with Object prototype keys like `constructor`, which made the assignee selector receive a non-array member list in issue rows.

Use a Map for the project member index so slug lookups are data lookups instead of object property access. Adds coverage for the issue-list path that was crashing.

Co-authored-by: Codex GPT-5 <noreply@openai.com>
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 5, 2026
Keep memberList required at issue-list component boundaries while still allowing the query result to be undefined during loading or from drawer-only entry points.

Co-authored-by: Codex GPT-5 <noreply@openai.com>
Comment on lines +59 to +61
const projectMembers = result.get(project) ?? [];
projectMembers.push(member.user);
result.set(project, projectMembers);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i tried but it doesn't work in jest yet, will need to figure out whats happening there

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in prod i do see the pollyfill already

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool. no need to rush this little thing

@shashjar shashjar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is pretty funny

drawerKey: 'supergroup-drawer',
});
openDrawer(
() => <SupergroupDetailDrawer memberList={undefined} supergroup={supergroup} />,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think memberList can just be omitted here since it's optional for SupergroupDetailDrawer, right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or should that one be updated to be required alongside the SupergroupIssueList props?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll just try to not touch super group drawer for now, going to delete soon

@scttcper scttcper enabled auto-merge (squash) June 5, 2026 18:27
@scttcper scttcper merged commit a56571a into master Jun 5, 2026
75 checks passed
@scttcper scttcper deleted the scttcper/member-map-index branch June 5, 2026 18:27
scttcper added a commit that referenced this pull request Jun 5, 2026
Follow-up from #116996 prevents a page crash when typing `constructor:`
scttcper added a commit that referenced this pull request Jun 8, 2026
Follow-up from #116996 

Filtering on 'constructor' would collide with object properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants