1

Edit: Moved the getMetadata() call out of render, as suggested by @Robin Zigmond. Still having trouble getting the proper value from getMetadata() returned, but that's different from my original question. Updating code just for reference.

Quick background: I have a fair amount of experience with shell scripting and stuff like Perl and PHP, but javascript and especially the libraries on top of it like React feel very foreign to me. Forcing myself to develop this with React to teach myself something new, so if you see any bad practices, feel free to suggest improvements!

That said, I'm trying to write a simple app that:

  1. Prompts for search parameter
  2. Runs query against 3rd party service that returns json results
  3. For each returned element, run additional query to get more details
  4. Display tabular results

I have 1, 2, and 4 generally worked out, but struggling with 3. Here's what I have so far, with less important code snipped out:

class VGSC_Search extends React.Component {
    constructor() {
        super();
        this.state = {
            submitted: false,
            platform: '',
            games: [],
            metadata: [],
            files: [],
        };
        this.handleChange = this.handleChange.bind(this);
        this.handleSubmit = this.handleSubmit.bind(this);
    }

    handleChange(event) {
        this.setState({platform: event.target.value});
    }

    handleSubmit(event) {
        this.setState({submitted: true});
        <SNIP - vars>
        fetch(encodeURI(searchurl + query + fields + sort + rows))
            .then(result => result.json())
            .then(data => this.setState({
                games: data.response.docs.map(game => ({
                    identifier: game.identifier,
                    title: game.title,
                    creator: game.creator,
                    year: game.year,
                    uploader: this.getMetadata(game.identifier),
                }))
            }));
        event.preventDefault();
    }

    getMetadata(id) {
        <SNIP - vars>
        fetch(encodeURI(metadataurl + id + metadatainfo))
            .then(result => result.json())
            .then(data => this.setState({metadata: data.response}));
    }

    renderResults() {
        const {games} = this.state;
        const {metadata} = this.state;

        return (
            <SNIP - table header>
                <tbody>{games.map(game =>
                    <tr key={game.identifier}>
                        <td><a href={'https://archive.org/details/' + game.identifier}>{game.title}</a></td>
                        <td>{game.creator}</td>
                        <td>{game.year}</td>
                        <td>{game.uploader}</td>
                    </tr>
                )}</tbody>
            </table>
        );
    }

    render(){
        return (
            <div>
            <SNIP - form>
            <br/>
            {this.state.submitted && this.renderResults()}
            </div>
        );
    }
}

My problem is with that Uploader field, which should run getMetadata() on the given identifier and return the name of the uploader. I'm having trouble figuring out how to reference the result, but my biggest problem is actually that my browser keeps running getMetadata() on all displayed items in an endless loop. Eg, from the Developer Tools log:

XHR GET https://archive.org/metadata/4WheelThunderEuropePromoDiscLabel/metadata/uploader [HTTP/1.1 200 OK 105ms]
XHR GET https://archive.org/metadata/AirJapanCompleteArtScans/metadata/uploader [HTTP/1.1 200 OK 279ms]
XHR GET https://archive.org/metadata/AeroWings-CompleteScans/metadata/uploader [HTTP/1.1 200 OK 287ms]
XHR GET https://archive.org/metadata/BioCodeVeronicaLEDCT1210MNTSCJ/metadata/uploader [HTTP/1.1 200 OK 279ms]
XHR GET https://archive.org/metadata/Biohazard2ValuePlusDreamcastT1214MNTSCJ/metadata/uploader [HTTP/1.1 200 OK 282ms]
XHR GET https://archive.org/metadata/4WheelThunderEuropePromoDiscLabel/metadata/uploader [HTTP/1.1 200 OK 120ms]
XHR GET https://archive.org/metadata/AirJapanCompleteArtScans/metadata/uploader    [HTTP/1.1 200 OK 120ms]
<SNIP>

The first search returns 5 results, and getMetadata() is run correctly on those five results, but note that it starts repeating. It'll do that endlessly until I reload the page.

I'm guessing that has something to do with running getMetadata() inside a function that's being rendered, but I'm not sure why, and having trouble thinking of a good alternate way to do that.

Can anyone explain why I'm seeing this behavior, and (hopefully) offer a suggestion on how to properly implement this?

Thanks!

5
  • It's because you're running getMetadata inside render (or rather inside a function that's called from render) - and that results in changing state, which causes a rerender, and so on in an endless loop. It's bad practice to call any function which cause any "side effects" from within render - render should simply determine the output given the component's props and state. I'm not sure how to fix your component because it's not clear when you want getMetadata to run. Typically such data-fetcing is done inside componentDidMount and/or componentDidUpdate. Commented Jul 12, 2020 at 20:35
  • Yeah, that makes sense. I basically want to run getMetada() for each element in the dataset returned by handleSubmit(). Does it make sense to do that in componentDidUpdate? That seems to fire literally on every change (even with each character typed into the input box), so it seems like I'd have a similar issue with the queries being run multiple times instead of once per element. Commented Jul 12, 2020 at 21:49
  • While componentDidUpdate does indeed run on every state change, usually one avoids infinite loops by using an if statement to only update stuff if particular things have changed. Anyway, it strikes me here that, if you want to run getMetadata in response to what happens in handleSubmit, you should just call it from that method. Commented Jul 12, 2020 at 21:56
  • Agreed. I messed with that earlier, but had trouble figuring out how to edit the json array. I think I got it now, and see my 5 subqueries running with no repeat, but need to figure out the syntax to have getMetadata() return the value. Will update my code to reflect current state. Since you answered my primary question about WHY it loops through those subqueries, if you basically past that as an answer I'll be happy to accept it. Thanks for the guidance. Commented Jul 12, 2020 at 22:25
  • Thanks, I will post as an answer. Commented Jul 12, 2020 at 22:37

1 Answer 1

1

The infinite loop happens because you're running getMetadata inside render (or rather inside a function that's called from render) - and that results in changing state, which causes a rerender, and so on in an endless loop.

It's bad practice in any situation to call any function which cause any "side effects" from within render - render should simply determine the output given the component's props and state. Typically data-fetching like you're doing is done inside componentDidMount and/or componentDidUpdate. However in this case, where you appear to need to fetch additional data based on the first response in handleSubmit, it seems that you need to call getMetadata from within the final .then callback of that function.

After your latest edit, I can see the problem with the approach you tried here. this.getMetadata doesn't actually return anything. To fix it, you can return the Promise returned by fetch:

getMetadata(id) {
    <SNIP - vars>
    return fetch(encodeURI(metadataurl + id + metadatainfo))
               .then(result => result.json())
               .then(data => this.setState({metadata: data.response}));
}

and then use asyc/await inside handleSubmit, with Promise.all to manage the array:

    fetch(encodeURI(searchurl + query + fields + sort + rows))
        .then(result => result.json())
        .then(async data => {
            const games = await Promise.all(data.response.docs.map(async game => ({
                identifier: game.identifier,
                title: game.title,
                creator: game.creator,
                year: game.year,
                uploader: await this.getMetadata(game.identifier),
            })));
            this.setState({ games });
        });
Sign up to request clarification or add additional context in comments.

4 Comments

Thanks so much! The async/await stuff is honestly going way over my head, but let me study that a bit more and see if I can make sense of it. I copied it over and I'm getting a 'games is not defined' error right now, but I'm probably missing something basic.
apologies, since I could see your edit when I started the answer and could see what was wrong with it, I thought I'd better try to give a way to fix it. But the code is a little complex, and it's more than possible I missed something - apologies if I did! (Although I just looked briefly again and can't see why games would not be defined in the code I gave.)
Found it - had this.setState({ games }); outside the closing }); for that stanza. Silly mistake. If you don't mind me bothering you once more, I'm now getting "TypeError: Argument of Promise.all is not iterable". The data returned by fetch should be json, and my understanding is that should be of type array. Any idea off the top of your head why that may be failing?
Oops, silly mistake while moving code about - I'd accidentally passed a function to Promise.all. Should be fixed now.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.