State being set before promise call is finished

Question

As the title suggests I am trying to use a promise to set the state of my react component. Simple enough, right? Maybe not. Well the issue arises when I am trying to make a subsequent ajax call in a promise.

The code is below. As you can see I am trying to map over that data that I get back and create a new object. In the creation of that object there is another promise call that is supposed to set one of the fields. All of this works correctly however the state is being set before the second promise is finished and that field is unavailable to me.

I've tried using async/await to get it to wait on the call to be finished but I haven't been successful in this approach either.

Any suggestions you might have would be greatly appreciated.

I am making the call in the ComponentDidMount method:

public componentDidMount(): void {
    this._spApiService
      .getListItems(this.props.context, "Company News")
      .then((spNewsStories: SpNewsStory[]) => {
        return spNewsStories.map((newsStory: SpNewsStory) => {
          return new AdwNewsStory(newsStory, this.props.context);
        });
      })
      .then((adwNewsStories: AdwNewsStory[]) => {
        this.setState({
          topStories: adwNewsStories,
       });
    });
  }

And here is the AdwNewStory Class that makes the second ajax call:

import { SpNewsStory } from "./SpNewsStory";
import { ISpApiService } from "../../interfaces/ISpApiService";
import SpApiService from "../../services/SpApiService";
import { WebPartContext } from "../../../node_modules/@microsoft/sp-webpart- 
base";
import { SpAttachment } from "../SpAttachment";
import IEnvironmentService from "../../interfaces/IEnvironmentService";
import EnvironmentService from "../../services/EnvironmentService";
import { IAdwDateTimeService } from "../../interfaces/IAdwDateTimeService";
import AdwDateTimeService from "../../services/AdwDateTimeService";

class AdwNewsStory {
  public id: number;
  public title: string;
  public publishDate: string;
  public storySummary: string;
  public storyLink: string;
  public windowTarget: string;
  public imageUrl: string;
  public imageAlternativeText: string;
  public attachments: boolean;

  private _spApiService: ISpApiService;
  private _context: WebPartContext;
  private _environmentService: IEnvironmentService;
  private _adwDateTimeService: IAdwDateTimeService;

  constructor(spNewsStory: SpNewsStory, context?: WebPartContext) {
    this._spApiService = new SpApiService();
    this._context = context;
    this._environmentService = new EnvironmentService();
    this._adwDateTimeService = new AdwDateTimeService();
    this.buildAdwNewsStory(spNewsStory);
  }

  private buildAdwNewsStory = (spNewsStory: SpNewsStory): void => {
    this.id = spNewsStory.Id;
    this.title = spNewsStory.Title;
    this.publishDate = this.setPublishDate(spNewsStory.PublishDate);
    this.storySummary = spNewsStory.StorySummary;
    this.storyLink = spNewsStory.Link.Description;
    this.windowTarget = spNewsStory.WindowTarget;
    this.imageAlternativeText = spNewsStory.ImageAlternateText;
    this.attachments = spNewsStory.Attachments;
    if (this.attachments) {
      this.setImageUrl();
    }
  };

  private setImageUrl = (): void => {
      this._spApiService.getListItemAttachments(this._context, "Company News", this.id).then(attachments => {
      const siteUrl: string = this._environmentService.getSiteUrl();
      const attchmentUrl: string = `${siteUrl}/Lists/Company%20News/Attachments/${this.id}/${attachments[0].FileName}`;
      this.imageUrl = attchmentUrl;
    });
  };

 private setPublishDate = (dateString: string) => {
    const publishDate: Date = new Date(dateString);
    return `${this._adwDateTimeService.getMonthName(publishDate.getMonth())} ${publishDate.getDate()}, ${publishDate.getFullYear()}`;
 };
}

export default AdwNewsStory;

Answer 1

You have a couple problems here. First off:

private setImageUrl = (): void => {
    // You aren't returning this promise here, so there's no way for
    // calling code to get notified when the promise resolves
    this._spApiService.getListItemAttachments(this._context, "Company News", this.id).then(attachments => {
        const siteUrl: string = this._environmentService.getSiteUrl();
        const attchmentUrl: string = `${siteUrl}/Lists/Company%20News/Attachments/${this.id}/${attachments[0].FileName}`;
        this.imageUrl = attchmentUrl;
    });
};

The bigger problem, however, is that you're calling that method from the constructor. You really don't want constructors to make async calls. While there's no technical reason not to, there's really no way to practically return that promise to the caller. This leads to bugs like the one you're experiencing.

I would suggest moving the buildAdwNewsStory() method out of the constructor, and calling that separately as it involves an async call.

class AdwNewsStory {
    // ... fields...
    constructor(context?: WebPartContext) {
        this._spApiService = new SpApiService();
        this._context = context;
        this._environmentService = new EnvironmentService();
        this._adwDateTimeService = new AdwDateTimeService();
    }

    public buildAdwNewsStory(spNewsStory: SpNewsStory): Promise<void> {
        this.id = spNewsStory.Id;
        this.title = spNewsStory.Title;
        this.publishDate = this.setPublishDate(spNewsStory.PublishDate);
        this.storySummary = spNewsStory.StorySummary;
        this.storyLink = spNewsStory.Link.Description;
        this.windowTarget = spNewsStory.WindowTarget;
        this.imageAlternativeText = spNewsStory.ImageAlternateText;
        this.attachments = spNewsStory.Attachments;
        if (this.attachments) {
            return this.setImageUrl();
        } else {
            return Promise.resolve();
        }
    }
    ...
    private setImageUrl() {
        return this._spApiService.getListItemAttachments(this._context, "Company News", this.id)
            .then(attachments => {
                const siteUrl: string = this._environmentService.getSiteUrl();
                const attchmentUrl: string = `${siteUrl}/Lists/Company%20News/Attachments/${this.id}/${attachments[0].FileName}`;
                this.imageUrl = attchmentUrl;
            });
    }
}

Then from your componentDidMount:

public componentDidMount(): void {
    this._spApiService
        .getListItems(this.props.context, "Company News")
        .then((spNewsStories: SpNewsStory[]) =>
            spNewsStories.map((newsStory: SpNewsStory) => {
                var story = new AdwNewsStory(this.props.context);
                return story.buildAdwNewsStory(newsStory);
            })
        )
        .then((adwNewsStories: AdwNewsStory[]) => {
            this.setState({
                topStories: adwNewsStories,
            });
        });
}

As a side note, this code could possibly look quite a bit cleaner if you moved to async/await instead of using the Promise methods directly (.then/.catch)

Answer 2

The setState is firing before your promise is resolved in this case, because the constructor of your class will return immediately, despite calling an asynchronous action.

You could rethink your setup and provide your AdwNewStory instance with the information it needs when creating the instance of it, allowing you better control of the flow of actions, or you could pass a callback to adwNewStory and allow that to tell you when a story has been created, however that will lead you to anti-patterns.