ref(stores): Make ProjectsStore useLegacyStore compatible #29247
Conversation
db38e88 to
bf290ac
Compare
size-limit report
|
bf290ac to
aa92425
Compare
|
LGTM |
|
LGTM Actually, nit, but consider providing a brief description here so we know what this change actually does |
Also removes the withProjectsSpecified HoC. There's a very marginal performance regression here since the HoC handled only re-rendering when the specified projects changed. This isn't a huge concern, since projects rarely are changing anyway.
aa92425 to
99c328c
Compare
| project => project.isMember | ||
| ); | ||
| const specifiedProjects = specificProjectSlugs | ||
| ? projects.filter(project => specificProjectSlugs.includes(project.slug)) |
There was a problem hiding this comment.
This is basically exactly what the withProjectsSpecified HoC was doing
| loading: false, | ||
| })); | ||
| jest.spyOn(ProjectsStore, 'getAll').mockImplementation(() => initialData.projects); | ||
| jest.spyOn(ProjectsStore, 'isLoading').mockImplementation(() => false); |
There was a problem hiding this comment.
Both functions are called now in withProjects
Kind of annoying that this function mocked all of the store. I almost would have rather it just used the store
| * IssuesList ("child view") renders before a single project is enforced, | ||
| * will require refactoring views so that they depend on GSH enforcing a | ||
| * single project first IF they don't have required feature (and no project id | ||
| * in URL). |
There was a problem hiding this comment.
Just wrapping these a bit smaller to fit in 80ch windows
|
|
||
| wrapper = mountWithTheme( | ||
| <GlobalSelectionHeader | ||
| organization={initialData.organization} | ||
| shouldForceProject | ||
| forceProject={initialData.organization.projects[0]} |
There was a problem hiding this comment.
A lot of tests still use organization.projects but probably shouldn't anymore
| }); | ||
|
|
||
| // Force the withProjects HoC to re-render | ||
| ProjectsStore.trigger(); |
There was a problem hiding this comment.
This is pretty gross, but what we had here before where we mutated the state object was pretty bad too so 🤷
|
@matejminar could I get another review actually. I ended up removing the it also made fixing tests a bit easier |
No description provided.