apache / incubator-weex-loader

Apache License 2.0
65 stars 32 forks source link

fix an issue that cannot find the right babel-loader module path #51

Closed daxingplay closed 7 years ago

daxingplay commented 7 years ago

Recently I found an issue with weex-loader compiling .we files. I wrote a simple webpack config from scratch:

module.exports = {
  entry: {
    app: path.resolve('./src/app.we?entry=true'),
  },
  output: {
    path: path.resolve(__dirname, 'build'),
    filename: '[name].js'
  },
  module: {
    loaders: [
      {
        test: /\.js$/,
        loader: 'babel',
        exclude: /node_modules/
      },
      {
        test: /\.we(\?[^?]+)?$/,
        loaders: ['weex']
      },
      {
        test: /\.(png|jpe?g|gif|svg)(\?.*)?$/,
        loader: 'url',
        query: {
          limit: 10000,
        }
      }
    ]
  },
  plugins: [
    new webpack.optimize.UglifyJsPlugin({
      compress: {
        warnings: false
      }
    }),
  ]
};

then I used tnpm to install modules with webpack 1.14.0, weex-loader 0.4.4, babel-loader 6.4.0 and some other related packages. When I run build, there came an error:

ERROR in ./src/app.we?entry=true
Module not found: Error: Cannot resolve 'file' or 'directory' ../node_modules/.npminstall/babel-loader in /Users/daxingplay/xxx/xxx/src
 @ ./src/app.we?entry=true 3:22-730

I tried to delete node_modules and re-run tnpm install many times, the error was still there. I looked into my node_modules and found that my babel-loader was in /Users/daxingplay/xxx/xxx/node_modules/.npminstall/babel-loader/6.4.0/babel-loader/. After debugging weex-loader, I found this issue was related with the function loadBabelModule:

function loadBabelModule(moduleName) {
  try {
    var _path = require.resolve(moduleName);
    return _path.slice(0, _path.indexOf(moduleName) + moduleName.length);
  } catch (e) {
    return moduleName;
  }
}

I modified it to

function loadBabelModule(moduleName) {
  try {
    return require.resolve(moduleName);
  } catch (e) {
    return moduleName;
  }
}

Then the error was gone.

After reviewing #31 , PR #26 and this commit, I found that maybe I cannot simply use require.resolve to load babel modules, so I tried to change indexOf to lastIndexOf, the error was gone as well.

function loadBabelModule(moduleName) {
  try {
    var _path = require.resolve(moduleName);
    return _path.slice(0, _path.lastIndexOf(moduleName) + moduleName.length);
  } catch (e) {
    return moduleName;
  }
}

I guess my problem has some relations with node 6.10 and npm 3.10.10 or tnpm 4.19.4, it installs node_modules differently and put babel-loader under sub-folder with a version number. I'm not quite familiar with what changed in npm or tnpm, so I'm not sure my PR is right. You guys can judge that. πŸ˜†

daxingplay commented 7 years ago

Sorry that I cannot upgrade my existing project to .vue files because there are still some users especially Android users using old versions of our App with an older version of Weex SDK which not supports vue-framework.

Hanks10100 commented 7 years ago

Thanks for fixing this. I think we should use lastIndexOf instead of indexOf here. You have such a patience and learned many details about weex-loader. Great job. πŸ‘

By the way, I also think path is not appropriate since it's a built-in module of Node.js. filePath maybe more reliable. You can change it passingly.

daxingplay commented 7 years ago

Thanks for your advice. I think filePath is much better as well.

daxingplay commented 7 years ago

Hi, is this PR ready to merge? Thanks

Hanks10100 commented 7 years ago

Sorry for the delay, I have merged this PR.