Using Rollup on Windows by vanruesc · Pull Request #117 · rollup/rollup

vanruesc · 2015-09-18T11:51:45Z

Hi,

I started using Rollup recently, but encountered some issues that were initially preventing me from using its full range of capabilities. (I'm using Windows because I'm a bit of a masochist.)

When there is an external module to load like so:

// src/main.js
import Evil from "@pandora/box";

It fails with this error:

>> rollup -f cjs -i src/main.js -o build/main.js

Cannot read property '0' of null
TypeError: Cannot read property '0' of null
    at Object.defaultExternalResolver [as resolveExternal]
    (F:\Sourcecode\JavaScript\evil\node_modules\rollup\dist\rollup.js:2692:40)
...

Browserify + Babelify handles this without problems. JSHint doesn't shout at me either.

The Rollup CLI creates valid bundles for me on Windows when there are no modules to import. So I figured that there might be a Windows-specific bug somewhere in resolveId and thus in path logic.

I then tried the JavaScript API instead of the CLI, but got the same Cannot read property '0' of null error at first. However, after overriding the defaultResolver it worked even though I wasn't using any black magic. Here is a Gist if you're interested in the details.

This helped me find the problematic code:

// src/utils/path.js
export const absolutePath = /^(?:\/|(?:[A-Za-z]:)?\\)/;

I saw that on Windows, the importer parameter in resolveExternal() contains strings like these:

F:/Sourcecode/JavaScript/myProject/src/main.js

The obvious solution was to add normal slashes to the regular expression:

/^(?:\/|(?:[A-Za-z]:)?[\\|\/])/

When I tried to build the project myself to test this, it straight out faceplanted like an overly enthusiastic skateboarder. It's strange that nobody has pointed this out before, but Rollup cannot be built on Windows at the moment. The main reason for this is that it uses itself internally during the build process. Since I can't build it locally like this, I directly modified node_modules/rollup/dist/rollup.js in my projects and got it to work. The CLI works on Windows with this little change.

If you want to try building Rollup yourself on Windows, you could use this config. Appveyor is pretty similar to travis-ci.

Windows didn't like this regular expression. Added normal slashes to the mixture.

lukeapage · 2015-09-18T18:40:42Z

dirname has the same problem, it ignores : and I think that's why it fails to build.

but making this simple change:

/(:?\/|\\)[^\/\\]+$/

won't fix it. I actually think the logic is a bit broken, I can't see how it will work in the failing case.. I think possibly it might be better for...

while ( dir !== root ) {

to be..

while ( dir !== "." ) {

but really this code needs unit tests and then cases for each OS format.

Have you considered ditching all this code and using : https://github.com/substack/node-resolve ?

lukeapage · 2015-09-19T08:47:59Z

I'm working on a PR that fixes the tests on windows.

Its a chicken and egg situation - because rollup uses rollup, a new release is required before rollup will build on windows.. I've worked around it by copying rollup to node_module/rollup.

@vanruesc I'll take your 2nd commit. I still think testing with appveyor is essential, but it should go in once a release with windows fixes is in.

vanruesc · 2015-09-19T12:16:03Z

I missed your issue report by a couple of hours yesterday. Good to know that it's being worked on!

With the absolutePath RegExp change in this PR, I haven't encountered any other problems yet, but I agree that the logic might be lacking. The important thing for me was to get it to work, even though I have to hack into node_modules/rollup for now.

Have you considered ditching all this code and using : https://github.com/substack/node-resolve ?

I assume this was addressed to @Rich-Harris ? In any case, I also think that the path resolution logic of Rollup could probably be simplified/fixed with node-resolve.

I'll take your 2nd commit. I still think testing with appveyor is essential, but it should go in once a release with windows fixes is in.

Okidoki.

lukeapage · 2015-09-19T12:30:41Z

I assume this was addressed to @Rich-Harris ?

yep

and Obviously everything i said is jyst my opinion.. will raise pr today hopefully, just got distracted...

Rich-Harris · 2015-09-19T17:27:34Z

Thanks @vanruesc, have merged this to add the appveyor.yml file. No luck so far, I get this error message:

I'm sure there's an obvious explanation, am looking into it...

lukeapage · 2015-09-19T17:37:39Z

I've never had that problem. comparing to the last one I setup (https://github.com/lukeapage/pngjs2/blob/master/appveyor.yml) I can't see any breaking differences..

http://www.appveyor.com/docs/lang/nodejs-iojs

It looks like it isn't finding the appveyor.yml file (or else interpretting it wrong) :S

Rich-Harris · 2015-09-19T17:39:58Z

Turned out it doesn't pick up the appveyor.yml file if it's a Git project rather than a GitHub project. It wouldn't allow me to add it as a GitHub project because Appveyor only shows a subset of your public repos... sigh. I had to use a different project then edit the repo URL – so the Appveyor project URL is https://ci.appveyor.com/project/Rich-Harris/rollup-starter-project, confusingly enough.

It still fails, of course, but in a more encouraging way :)

lukeapage · 2015-09-19T17:41:46Z

cool, you probably know this, but if not, thats just because rollup depends on rollup, so appveyor won't pass till there is a release

Rich-Harris · 2015-09-19T17:42:45Z

Yep – am just putting together the 0.16 changelog then I'll push it out and update the plugins. Gotta love this chicken and egg stuff

Rich-Harris · 2015-09-19T21:37:36Z

Progress report: it basically works on Windows AFAICT, but Appveyor gives it a fail because of some BS to do with newlines (which causes sourcemap mappings to be off by one, etc, etc). I've had as much Windows pain as I can deal with for one afternoon so I'll call this a partial victory and live to fight another day.

lukeapage · 2015-09-19T21:54:57Z

i think that might be caused by and fixed by this line in appveyor. yml

init:
  - git config --global core.autocrlf true

I have my own setting on false as I prefer unix line endings and the tests pass.

Rich-Harris · 2015-09-20T00:38:15Z

Cool, thanks @lukeapage. It fails for reasons unknown on node 0.10 and 0.12, but builds just fine on more recent versions.

lukeapage · 2015-09-20T05:45:04Z

I think you were unlucky with the last build - appveyor sometimes fails.. the builds fail at install points before where they got to in previous builds.

lukeapage · 2015-09-20T05:51:32Z

btw thanks alot for rollup, I think it is the future. I was previously using requirejs and amdclean to achieve a similar result, but with this I can be rid of require js, yay!

vanruesc · 2015-09-20T12:19:40Z

I think you were unlucky with the last build - appveyor sometimes fails.

Yes, I can confirm that. I experienced lots of stupid errors today as well. Gave me seizures 💢 But it worked eventually.

@Rich-Harris I made a clean fork of the latest version of Rollup and tried building it with Appveyor again. It succeeded right away. You could just give it another try by hitting the re-build button.

When I checked the build log I remembered that Rollup automagically runs npm test when it is installed (due to prepublish) which means it runs the tests twice. Line 26 in the current Appveyor.yml doesn't hurt, but could probably be removed:

  - npm test

On a side note: the project-specific Appveyor badge can be found under settings>badges.

vanruesc added 2 commits September 18, 2015 13:29

Added Appveyor config.

9749b34

Modified absolutePath RegExp.

989455f

Windows didn't like this regular expression. Added normal slashes to the mixture.

lukeapage mentioned this pull request Sep 18, 2015

Exception on windows node 0.12 - bad path handling #119

Closed

lukeapage mentioned this pull request Sep 19, 2015

Fix windows #122

Merged

Rich-Harris merged commit 989455f into rollup:master Sep 19, 2015

lukeapage mentioned this pull request Sep 25, 2015

appveyor / windows testing #145

Closed

3 tasks

Uh oh!

Uh oh!

Conversation

vanruesc commented Sep 18, 2015

Uh oh!

lukeapage commented Sep 18, 2015

Uh oh!

lukeapage commented Sep 19, 2015

Uh oh!

vanruesc commented Sep 19, 2015

Uh oh!

lukeapage commented Sep 19, 2015

Uh oh!

Rich-Harris commented Sep 19, 2015

Uh oh!

lukeapage commented Sep 19, 2015

Uh oh!

Rich-Harris commented Sep 19, 2015

Uh oh!

lukeapage commented Sep 19, 2015

Uh oh!

Rich-Harris commented Sep 19, 2015

Uh oh!

Rich-Harris commented Sep 19, 2015

Uh oh!

lukeapage commented Sep 19, 2015

Uh oh!

Rich-Harris commented Sep 20, 2015

Uh oh!

lukeapage commented Sep 20, 2015

Uh oh!

lukeapage commented Sep 20, 2015

Uh oh!

vanruesc commented Sep 20, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants