Skip to content
This repository has been archived by the owner on Dec 6, 2021. It is now read-only.

Stop ignoring errors inside vue.config #57

Merged
merged 7 commits into from
Nov 20, 2016
Merged

Stop ignoring errors inside vue.config #57

merged 7 commits into from
Nov 20, 2016

Conversation

neves
Copy link

@neves neves commented Nov 19, 2016

If I create a vue.config.js like this:

const webpack = require('webpack')
module.exports = {
  title: 'todo',
  resolve: true,
  open: false,
  plugins: [
    new webpack.DefinePlugin({
      VERSION: '"1.2.3"'
    })
  ]
}

It will fail to load config when webpack is not a dependency and vbuild is running globally as usual.
That's because it will raise a MODULE_NOT_FOUND error that is miss interpreted as config file not found, and nothing is printed.
This pull request try to fix that, but keep the default behavior to support .js and .json config files.

Please, let me know if I need to better explain myself

try {
fileConfig = require(_.cwd(configFilePath))
let resolvedConfigFilePath = _.cwd(configFilePath)
if ( fs.existsSync(resolvedConfigFilePath) ||
Copy link
Owner

Choose a reason for hiding this comment

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

we have path-exists installed, we can use that

Copy link
Owner

Choose a reason for hiding this comment

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

never mind, I'll fix this

let resolvedConfigFilePath = _.cwd(configFilePath)
if ( fs.existsSync(resolvedConfigFilePath) ||
fs.existsSync(resolvedConfigFilePath + '.js') ||
fs.existsSync(resolvedConfigFilePath + '.json') ) {
Copy link
Owner

@egoist egoist Nov 19, 2016

Choose a reason for hiding this comment

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

the configFilePath may have extension, should stop adding extension if it already ends with .js or .json

Copy link
Owner

Choose a reason for hiding this comment

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

Oops, I see, we don't need to check this.

Copy link
Owner

Choose a reason for hiding this comment

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

damn, I accidentally clicked approved changes button 😐

fileConfig = require(_.cwd(configFilePath))
if (pathExists.sync(configFilePath) ||
pathExists.sync(configFilePath + '.js') ||
pathExists.sync(configFilePath + '.json')) {
Copy link
Owner

Choose a reason for hiding this comment

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

maybe we can use something like:

const existingConfigFilePath = yield Promise
  .all(['', '.js', '.json'].map(ext => pathExists(`${configFilePath}${ext}`)))
  .then(result => getExistingFilePath(result))

fileConfig = require(existingConfigFilePath)

Could be faster?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure if for just 3 sync calls, this async solution would be faster enough to justify that complexity.

@egoist egoist merged commit 9b8b3f7 into egoist:master Nov 20, 2016
@neves neves deleted the patch-2 branch November 21, 2016 03:59
throrin19 added a commit to throrin19/poi that referenced this pull request Jan 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants