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

fix(v2): force terminate building if client bundle failed #2345

Merged
merged 2 commits into from
Mar 8, 2020

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Feb 29, 2020

Motivation

Resolve #2311.

Server build uses the WaitPlugin plugin, which waits until a client bundle is created. This wait is infinite, so if the building of client bundle fails, the build process enters idle mode.

// Wait until manifest from client bundle is generated
new WaitPlugin({
filepath: path.join(generatedFilesDir, 'client-manifest.json'),
}),

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

  • In a MDX file try to import unresolvable dependency e.g.:
---
id: mdx
title: MDX page
---

import Blah from 'Blah';

Heading...

Prior to this fix, the build process became idle and needed to be killed (for example, by pressing CTRL + C)

Now the build process in case of an error in the client bundle terminated immediately.

Results:

Before (idle) After (forced terminated)
снимок_2 снимок

Related PRs

(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.)

@lex111 lex111 added the pr: bug fix This PR fixes a bug in a past release. label Feb 29, 2020
@lex111 lex111 added this to the v2.0.0-alpha.44 milestone Feb 29, 2020
@lex111 lex111 requested a review from yangshun February 29, 2020 22:41
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Feb 29, 2020
@lex111 lex111 force-pushed the lex111/fix-server-bundle-idle branch from 5e3582b to 390916c Compare February 29, 2020 22:43
@@ -30,6 +31,26 @@ export function createClientConfig(props: Props): Configuration {
runtimeChunk: true,
},
plugins: [
// Plugin to force terminate building if errors happened in the client bundle
Copy link
Contributor Author

@lex111 lex111 Feb 29, 2020

Choose a reason for hiding this comment

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

Perhaps this tiny plugin needs to be created in a separate file or class (in same file), but not sure about it. But feel free to change that. Something like ClientBundleCheckerPlugin??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix(v2) -> chore(v2) ?

Not sure exactly how the type of commit here accurately reflects the situation...

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this tiny plugin needs to be created in a separate file or class (in same file), but not sure about it. But feel free to change that. Something like ClientBundleCheckerPlugin??

I feel like you are right here. It can be made a separate plugin. Don't wish to take away your thunder and all your hard work on this but I can try do that if you want? :)

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Feb 29, 2020

Deploy preview for docusaurus-2 ready!

Built with commit d2e4f14

https://deploy-preview-2345--docusaurus-2.netlify.com

@facebook facebook deleted a comment from docusaurus-bot Feb 29, 2020
@lex111 lex111 force-pushed the lex111/fix-server-bundle-idle branch from 390916c to 0e81395 Compare March 6, 2020 20:12
@@ -4,6 +4,8 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import chalk from 'chalk';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although it is recommended that we use the CommonJS module format (import chalk = require('chalk');) to import chalk, but when using this style, tests will fail with such an error:

 `import =` is not supported by @babel/plugin-transform-typescript
Please consider using `import <moduleName> from '<moduleName>';` alongside Typescript's --allowSyntheticDefaultImports option.

It is OK?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this is a known limitation. That is fine then :)

@yangshun yangshun merged commit c9ace3b into master Mar 8, 2020
@yangshun yangshun deleted the lex111/fix-server-bundle-idle branch March 8, 2020 15:20
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.

[V2] docusaurus build becomes idle instead of throwing an error
5 participants