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

feat: add async load of config #1618

Closed

Conversation

romulocintra
Copy link

Summary

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

fix #1185

Needs more convenient tests

@flozero
Copy link
Collaborator

flozero commented Sep 5, 2019

Thank's for your work @romulocintra ! I know it's been a while since you create the pr.

I will have to check a bit about async stuff here. It's look good anyway.

@flozero flozero self-assigned this Sep 5, 2019
@flozero flozero added status: awaiting more context Need more context about this issue type: enhancement Request to enhance an existing feature status: core team assigned Core team member assigned version: 1.x Relates to version 1 of VuePress labels Sep 5, 2019
@@ -28,9 +28,12 @@ module.exports = function loadConfig (vuepressDir, bustCache = true) {
} else if (fs.existsSync(configTomlPath)) {
siteConfig = parseConfig(configTomlPath)
} else if (fs.existsSync(configPath)) {
siteConfig = require(configPath)
if (typeof require(configPath).then === 'function') {
siteConfig = require(configPath).then((config) => config)
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you can add a catch with a pretty error message it will be good for me.

use logs in shared utils package to log the error

Copy link
Author

Choose a reason for hiding this comment

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

I will

Copy link
Author

Choose a reason for hiding this comment

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

Hi @f3ltron not to much time to review and add a proper commit but updated with catch and logger.error message

@flozero flozero added need feedback Awaiting author response status: community assigned Community assigned and removed status: awaiting more context Need more context about this issue labels Sep 7, 2019
@flozero
Copy link
Collaborator

flozero commented Sep 10, 2019

tank's for your work ! 👍 i validate it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need feedback Awaiting author response status: community assigned Community assigned status: core team assigned Core team member assigned type: enhancement Request to enhance an existing feature version: 1.x Relates to version 1 of VuePress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support asynchronous config for generated sidebar
2 participants