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

Doesn't build on windows #7

Closed
rxaviers opened this issue Oct 1, 2015 · 23 comments
Closed

Doesn't build on windows #7

rxaviers opened this issue Oct 1, 2015 · 23 comments

Comments

@rxaviers
Copy link
Owner

rxaviers commented Oct 1, 2015

Original: globalizejs/globalize#516

Unless I am missing something, I don't think globalize-webpack-plugin would build on windows... I am thinking regular expressions might be the problem:

GlobalizeCompilerHelper.prototype.getModuleFilepath = function(request) {
  return path.join(this.tmpdir, request.replace(/.*!/, "").replace(/[/?" ]/g, "-"));
};

ERROR in ./app/index.js
Module parse failed: C:\app-npm-webpack\app\index.js ENOENT, no such file or directory 'C:\app-npm-webpack.tmp-globalize-webpack\C:\app-npm-webpack\app\index.js'
You may need an appropriate loader to handle this file type.
Error: ENOENT, no such file or directory 'C:\app-npm-webpack.tmp-globalize-webpack\C:\app-npm-webpack\app\index.js'

Jörn has added this comment:

Classic Windows path issue. Should be easy enough to fix. While at it, set up CI on Windows? http://www.appveyor.com/ makes that reasonably easy.

@rxaviers
Copy link
Owner Author

rxaviers commented Oct 1, 2015

Hi Rafael,

Yes, I would like to try to make it work on Windows as this is our dev
environment. I'd appreciate if you could give me some guidance.
I tried a quick&dirty fix,

GlobalizeCompilerHelper.prototype.getModuleFilepath = function(request) {
return path.join(this.tmpdir, request.replace(/.*!/,
"").replace(/[/?" :]/g, "-"));
};

but it goes one step farther and fails at

@anaritsin, your change seems correct to me. One more regexp will need update as well:

- compiler.apply(new SkipAMDPlugin(/(^|\/)globalize($|\/)/));
+ compiler.apply(new SkipAMDPlugin(/(^|[/\\])globalize($|[/\\])/));

@warstick
Copy link

warstick commented Oct 2, 2015

@rxaviers
As you suggested i followed following steps.

  1. Changed the getModuleFilepath in globalizecompilerhelper.js.

GlobalizeCompilerHelper.prototype.getModuleFilepath = function(request) {
return path.join(this.tmpdir, request.replace(/.*!/,
"").replace(/[/?" :]/g, "-"));
};

  1. Change complier.apply in DevlopmentModePlugin.js.

compiler.apply(new SkipAMDPlugin(/(^|/)globalize($|/)/));

My Output:
error

Am i following correct way to solve this issue.?

@rxaviers
Copy link
Owner Author

rxaviers commented Oct 2, 2015

GlobalizeCompilerHelper.prototype.getModuleFilepath = function(request) {
return path.join(this.tmpdir, request.replace(/.*!/,
"").replace(/[/?" :]/g, "-"));
};

It's missing the \ char as well. So:

GlobalizeCompilerHelper.prototype.getModuleFilepath = function(request) {
return path.join(this.tmpdir, request.replace(/.*!/, "").replace(/[/?" :\\]/g, "-"));
};
  1. You haven't changed anything there? Use
compiler.apply(new SkipAMDPlugin(/(^|[/\\])globalize($|[/\\])/));

@anaritsin
Copy link

I tried the

 compiler.apply(new SkipAMDPlugin(/(^|[/\\])globalize($|[/\\])/));

but it did not work for me.

Also if I want to build it in 'production' mode, should not it be changed in the ProductionModePlugin ?

@warstick
Copy link

warstick commented Oct 2, 2015

@rxaviers

Thanks it worked. but i got some other error.

error-1

For this should i need to install globalize-runtime module?

@rxaviers
Copy link
Owner Author

rxaviers commented Oct 2, 2015

Also if I want to build it in 'production' mode, should not it be changed in the ProductionModePlugin ?

@anaritsin Yeap.

@rxaviers
Copy link
Owner Author

rxaviers commented Oct 2, 2015

@warstick, it seems SkipAMDPlugin didn't work. It could be related to what @anaritsin said above.

It seems like you both are working on the same fix. If you create a PR you both could work together in the solution. I'm happy to assist as time permits.

@anaritsin
Copy link

@warstick @rxaviers
this is exactly where i got stuck - SkipAMDPlugin did not work. And this is what I tried:

compiler.apply(
// Skip AMD part of Globalize Runtime UMD wrapper.
//globalizeSkipAMDPlugin = new SkipAMDPlugin(/(^|\/)globalize($|\/)/),
  globalizeSkipAMDPlugin = new SkipAMDPlugin(/(^|[/\\])globalize($|[/\\])/),

// Replaces `require("globalize")` with `require("globalize/dist/globalize-runtime")`.
//new NormalModuleReplacementPlugin(/(^|\/)globalize$/, "globalize/dist/globalize-runtime"),
  new NormalModuleReplacementPlugin(/(^|[/\\])globalize$/, "globalize/dist/globalize-runtime"),

// Skip AMD part of Globalize Runtime UMD wrapper.
  //new SkipAMDPlugin(/(^|\/)globalize-runtime($|\/)/)
new SkipAMDPlugin(/(^|[/\\])globalize-runtime($|[/\\])/)

);

@rxaviers
Copy link
Owner Author

rxaviers commented Oct 2, 2015

On your local node_modules/skip-amd-webpack-plugin/index.js, inspect this.state.current.request by adding a console.log(this.state.current.request) at https://github.com/rxaviers/skip-amd-webpack-plugin/blob/master/index.js#L23 and https://github.com/rxaviers/skip-amd-webpack-plugin/blob/master/index.js#L30. Let's see how this is in Windows. It's also good to inspect the result of self.requestRegExp.test(this.state.current.request).

@anaritsin
Copy link

@rxaviers
the L23 is this:
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime.js
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\number.js
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\message.js
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\currency.js
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\date.js
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\relative-time.js
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\plural.js

the L30 - never executes

@rxaviers
Copy link
Owner Author

rxaviers commented Oct 2, 2015

The regexp should work.

/(^|[/\\])globalize-runtime($|[/\\])/.test("C:\\app-npm-webpack\\node_modules\\globalize\\dist\\globalize-runtime\\message.js")
// > true

Could you inspect the result of self.requestRegExp.test(this.state.current.request) as well?

@anaritsin
Copy link

1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime.js
1 self.requestRegExp.test(this.state.current.request)==>> true
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\number.js
1 self.requestRegExp.test(this.state.current.request)==>> true
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\plural.js
1 self.requestRegExp.test(this.state.current.request)==>> true
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\message.js
1 self.requestRegExp.test(this.state.current.request)==>> true
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\date.js
1 self.requestRegExp.test(this.state.current.request)==>> true
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\relative-time.js
1 self.requestRegExp.test(this.state.current.request)==>> true
1 this.state.current.request==>> C:\app-npm-webpack\node_modules\globalize\dist\globalize-runtime\currency.js
1 self.requestRegExp.test(this.state.current.request)==>> true
1 this.state.current.request==>> C:\app-npm-webpack\.tmp-globalize-webpack\C--app-npm-webpack-app-index.js
1 self.requestRegExp.test(this.state.current.request)==>> false
1 this.state.current.request==>> C:\app-npm-webpack\.tmp-globalize-webpack\C--app-npm-webpack-app-index.js
1 self.requestRegExp.test(this.state.current.request)==>> false
Hash: 3cc7abe35aac638043d9
Version: webpack 1.12.2
Time: 2138ms

@box-turtle
Copy link

Anybody get this working?

@warstick
Copy link

Nope.

On Mon, Oct 19, 2015 at 6:10 PM, box-turtle [email protected]
wrote:

Anybody get this working?


Reply to this email directly or view it on GitHub
#7 (comment)
.

Thanks & Regards,

Manikanta Srirangam.

Contact No: +91-9866348399

@rxaviers
Copy link
Owner Author

rxaviers commented Nov 4, 2015

I'd be happy to merge a fix or to assist finding one.

@aruberto
Copy link
Contributor

I was able to get past the globalize-runtime/module not found issues by disabling AMD on the tmp files themselves. I added

new SkipAMDPlugin(new RegExp(this.tmpdir.replace(/(?=[\/\\^$*+?.()|{}[\]])/g, "\\")))

to ProductionModePlugin, and webpack completes successfully:

image

However still think something is wrong since get following errors when opening the page in dist folder:

image

@rxaviers
Copy link
Owner Author

@aruberto does development mode work for you? Can you give more information about the 'call' of undefined error (e.g., what's in vendor.js line 51)?

aruberto added a commit to aruberto/globalize-webpack-plugin that referenced this issue Jan 19, 2016
initial changes to get webpack to complete successfully
@aruberto
Copy link
Contributor

Hey @rxaviers

I've put what I've done so far in https://github.com/aruberto/globalize-webpack-plugin/tree/windows-fix

Here are the dist files that get generated from the app-npm-webpack project using my modified globalize-webpack-plugin on Windows and UglifyJS disabled:

dist.zip

As mentioned I get following errors when opening index.html in chrome

image

vendor.js 51 is

/******/        // Execute the module function
/******/        modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);

As for development mode, it never gave webpack errors even before my changes, however visting localhost:8080/index.html gives following error:

image

It is pretty much what globalizejs/globalize#551 is complaining about

@aruberto
Copy link
Contributor

Doing a diff between files generated on Linux and Windows (using my modified plugin), I notice this is off with files in dist/i18n.

On Linux, the generated i18n files start with

} else if ( typeof exports === "object" ) {

        // Node, CommonJS
        module.exports = factory( __webpack_require__(2), __webpack_require__(4), __webpack_require__(5), __webpack_require__(6), __webpack_require__(7), __webpack_require__(8) );

and end with something like:

module.exports = __webpack_require__(2);

however on Windows the dist/i18n files start with

module.exports = factory( __webpack_require__(undefined), __webpack_require__(undefined), __webpack_require__(undefined), __webpack_require__(undefined), __webpack_require__(undefined), __webpack_require__(undefined) );

and end with

module.exports = __webpack_require__(undefined);

@aruberto
Copy link
Contributor

The issue with i18n files is the splits in utils.js and ProductionModePlugin.js

changing

.split( "/" )

to

.split( /[\/\\]/ )

fixes issues with i18n files and page loads correctly!

aruberto added a commit to aruberto/globalize-webpack-plugin that referenced this issue Jan 19, 2016
@aruberto
Copy link
Contributor

I've got the dev plugin working as well.

I've submitted #12 which should fix this issue and globalizejs/globalize#551

Tested using https://github.com/jquery/globalize/tree/master/examples/app-npm-webpack, seems like everything works correctly.

@rxaviers
Copy link
Owner Author

Awesome, just confirming is it all working on Windows now using your fix?

@aruberto
Copy link
Contributor

Yes dev and prod modes both work for me with https://github.com/jquery/globalize/tree/master/examples/app-npm-webpack project

aruberto added a commit to aruberto/globalize-webpack-plugin that referenced this issue Feb 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants