Is it okay to call an async function that is fetching initial data in the constructor of a JS class?

user_7260252 0
Viewed 659

I'm building an example app with Mobx and Mobx React Lite to learn how to use this state management library. The app needs to have questions loaded when the user visits the page. Is it okay to call the initial load of data in the constructor like this? Could this cause bugs somehow?

My concern is that it might fetch again but I'm not sure that might be the only concern I should have.

What other pattern would you suggest if this is risky?

My alternate idea is to grab it out of context when they click a "Begin Quiz" button and have a loading screen while it fetches the data. That is most likely how it should happen but I'm more or less just wondering if what I did is also fine.

import { observable, action } from "mobx";
import { getQuestions } from "../api/api";

export interface Question {
    category: string;
    type: string;
    question: string;
    correct_answer: string;
    incorrect_answers: string[];
}

export class TriviaStore {

    constructor() {
        // Is this bad?
        (async() => {
            await this.loadQuestions();
        })()
    }

    @observable questions: Question[] = [];
    @observable currentQuestion: number = 0;

    @action
    async loadQuestions() {
        let questions: Question[] = await getQuestions();
        this.questions = questions;
    }
    @action
    nextQuestion() {
        this.currentQuestion++;
    }
}

The store is only instantiated once in a context provider like so:

import React from 'react';
import { TriviaStore } from '../stores/TriviaStore';

/**
 * Store Types.
 * Add any new stores here so the context type is updated.
 */
type RootStateContextValue = {
    triviaStore: TriviaStore;
}

const RootStateContext = React.createContext<RootStateContextValue>({} as RootStateContextValue);

/**
 * Stores
 * Use this area to create instances of stores to pass down.
 */
const triviaStore = new TriviaStore();

/**
 * Root State Provider
 * @returns global state context wrapper.
 */
export const RootStateProvider: React.FC<React.PropsWithChildren<{}>> = ({children}) => {
    // Finally pass new stores into the object below to send them to global state.
    return (
        <RootStateContext.Provider
            value={{
                triviaStore,
            }}
        >
            {children}
        </RootStateContext.Provider>
    );
}

export const useGlobalState = () => React.useContext(RootStateContext);
mobx typescript reactjs javascript
2 Answers

Initializing the store before all data is loaded is fine in my opinion. You can add the loading state directly to the store. Placing the async function as a method on the store is a good idea. Although I think the immediately executed async function wrapper has no effect and the store will be initialized before the questions are loaded. See this example:

@observable loading = true

constructor() {
  // Same as with your wrapper, constructor cannot be made async.
  this.loadQuestions()
}

@action
async loadQuestions() {
  let questions: Question[] = await getQuestions()
  // Newer versions of MobX will warn if runInAction missing after async call.
  runInAction(() => {
    this.questions = questions
    this.lading = false
  })
}

The not so good part in my opinion is mixing the MobX store with the React Context which I think isn't necessary as you can directly import the store from anywhere already.

user_31855450

Even though React will allow you to fetch some data and won't cause any bug in constructor, It is not suggested by React developers and it is not a good practice. You should fetch your data in componentDidMount() instead.

If your design requires several fetching after you load the page, you can try to do it in componentDidUpdate() and update your initial states.

user_97805590
Related