Skip to content
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

[BUG] Components don't re-render on React State update #3144

Open
tobi1220 opened this issue Mar 22, 2024 · 5 comments · May be fixed by #3339
Open

[BUG] Components don't re-render on React State update #3144

tobi1220 opened this issue Mar 22, 2024 · 5 comments · May be fixed by #3339

Comments

@tobi1220
Copy link

tobi1220 commented Mar 22, 2024

Describe the bug
The components don't re-render when a React state that's passed as a prop is updated. This happens for the React components in @microsoft/mgt-react as well as for the Web components in @microsoft/mgt-components (I tested both in a React app).

To Reproduce
You can test this with, for example, the file-list. (see full code below) Steps to reproduce the behavior:

  1. Init a new React app with CRA like so: yarn create react-app my-app --template typescript
  2. Add a stateful value and its update function like so: const [query, setQuery] = useState("");
  3. Add a <FileList /> in your App.tsx
  4. Add a function to the itemClick event that sets the current fileListQuery to the id of the clicked folder like so: <FileList fileExtensions={[""]} disableOpenOnClick itemClick={(e: CustomEvent<DriveItem>) => {setQuery(`/me/drive/items/${e.detail.id}/children`);}} fileListQuery={query}></FileList>

Expected behavior
The state update should trigger a re-render of <App /> and, by doing so, should update the MGT component.

Environment:

  • OS: Windows
  • Browser: Chrome
  • Framework: React
  • Context: MGT Core
  • Version: 4.1.0
  • Provider: new Msal2Provider({clientId: "redacted", authority: "redacted", redirectUri: "redacted", scopes: ["files.read"]});

Full code:

import {
  Login,
  FileList,
} from '@microsoft/mgt-react';
import { DriveItem } from '@microsoft/microsoft-graph-types';
import { useState } from 'react';

const App = () => {
  const [query, setQuery] = useState("");

  return (
    <div className='App'>
      <Login />

      <FileList /*key={query}*/ fileExtensions={[""]} disableOpenOnClick itemClick={(e: CustomEvent<DriveItem>) => {setQuery(`/me/drive/items/${e.detail.id}/children`);}} fileListQuery={query}></FileList> 
    </div>
  );
}

export default App;

Workaround
Add a key prop that is set to the respective state value to force a re-render every time this state is updated (although this is not recommended by React). In this case: key={query} (uncomment this in the full code provided above).

Additional context:
As mentioned above, this bug applies not only to the React- but also the Web components. I set up a new React app, registered the web components in it manually and set an attribute to a stateful value to test this.

@tobi1220 tobi1220 added bug Something isn't working Needs: Triage 🔍 labels Mar 22, 2024
@tobi1220 tobi1220 changed the title [BUG] [CORE] Components don't re-render on React State update [BUG] Components don't re-render on React State update Apr 4, 2024
@headinthebox
Copy link

Maybe a simpler repro is https://mgt.dev/?path=/story/components-mgt-file-list-html--open-folder-breadcrumbs
The file list does not re-render/

@tobi1220
Copy link
Author

Maybe a simpler repro is https://mgt.dev/?path=/story/components-mgt-file-list-html--open-folder-breadcrumbs
The file list does not re-render/

I didn't know this. Maybe this isn't even a bug then. But why does FileList not re-render in general?

@headinthebox
Copy link

Well, if you read the docs/comments, I am sure it is supposed to update file list

// render new file list <====
fileList.itemId = id;

also, if it does not replace the file list with the children of the thing you clicked on, the breadcrumbs make little sense.

Anyway, I found a workaround where I use mgt-get to get the list of files and then populate the files attribute of the mgt-file-list.

It is ugly and crappy as hell, but for now I can make progress.

@musale
Copy link
Contributor

musale commented Oct 22, 2024

@headinthebox @tobi1220 thank you looking into this issue and creating workarounds. I apologise for the late response and I acknowledge that this is a bug.

First, clicking on a folder does not re-render files in that folder. This is a bug as a folder with child items should open and re-render. The spec is also describing this behaviour https://github.com/microsoftgraph/microsoft-graph-toolkit/blob/main/specs/mgt-file-list.md

Second, the bread crumbs attach on the previous clicked item:
image

This is a bug, and the breadcrumbs should pile depending on the nesting of the clicked and currently viewed item. I will prioritise this in our next release cycle.

@headinthebox
Copy link

Awesome, thanks for looking into it. Of course I assumed MSFT code would be flawless, so I wasted quite some time looking for a bug in my own code ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Triage 🔍
Development

Successfully merging a pull request may close this issue.

4 participants