Skip to content

Comments

fix(v2): always exit after successful build#2443

Merged
yangshun merged 3 commits intofacebook:masterfrom
mohrash92:exit-on-successful-build
Mar 23, 2020
Merged

fix(v2): always exit after successful build#2443
yangshun merged 3 commits intofacebook:masterfrom
mohrash92:exit-on-successful-build

Conversation

@mohrash92
Copy link
Contributor

@mohrash92 mohrash92 commented Mar 22, 2020

Motivation

When running yarn build, a successful build does not always exit as highlighted in the following issue #2431. We can achieve this by exiting when the build has finished.

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

  1. Cloned repo and reproduced exiting problem as shown in [v2] yarn build hangs forever after build is completed successfully #2431
  2. Went into for v2 website for docusaurus and ran yarn and yarn build
  3. Replaced contents of build.js of hookstate with the one from docusaurus v2
  4. Ran yard build and the process exits instead of not exiting after build is successful
  5. Also ran yarn build --out-dir build/nested to ensure that the nested folder is generated and it works.

Screenshot 2020-03-22 at 14 55 57

Screenshot 2020-03-22 at 14 58 23

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@mohrash92 mohrash92 requested a review from yangshun as a code owner March 22, 2020 15:20
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 22, 2020
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Mar 22, 2020

@mohrash92 mohrash92 changed the title fix(v2): always exit after sucessfull build fix(v2): always exit after successful build Mar 22, 2020
Copy link
Contributor

@lex111 lex111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from async operations (e.g. setInterval in custom React component). But in any case, the building must finishes successfully therefore LGTM.

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants