Issues with @empty and sub-modules on paper.js

[lehni]

Hello,

I'm new to JSPM but was made aware of it though an issue with paper.js of which I am the maintainer: https://github.com/paperjs/paper.js/issues/1104

In short, the problem appears to be this: We have these settings in package.json:

  "browser": {
    "canvas": false,
    "jsdom": false,
    "source-map-support": false,
    "./dist/node/window.js": false,
    "./dist/node/extend.js": false
  },

And in our code, we ave this in one place, relative to the ./dist folder:

require('./node/window.js');

And that's where we seem to encounter the first problem: Although package.json tells the package manger not to load this module, it still gets loaded. Is it because it's not 100% the same path? They both point to the same file, from where the reference is made. And Webpack handles this correctly.

So then ./dist/node/window.js gets loaded, and this is where we encounter the second issue, which is even stranger. The file requires ./canvas.js, which requires a sub-module of jsdom:

require('jsdom/lib/jsdom/living/generated/utils');

This gets translated to:

'@empty/lib/jsdom/living/generated/utils'

And the browser can't load it. Shouldn't it be replaced with '@empty', since it 's a sub-module of jsdom?

But as I'm new to JSPM, I may be misinterpreting this situation completely.

It'd be nice to find a solution so we can support JSPM along with the other managers that already work.

[guybedford]

This seems to be a limitation of the jspm 0.16 approach. I think an override could fix this by adding individual overrides for the submodules perhaps:

{
  "browser": {
    "jsdon/lib/jsdom/living/generated/utils": false
  }
}

Such an override can be tested via jspm install npm:paper -o override.json where override.json contains all the browser values plus the extra ones as these do not mixin.

I just tested in jspm 0.17 and PaperJS seems to work correctly there.

Perhaps try the override and let me know if that approach can work out? Then we can get the override into the jspm registry (https://github.com/jspm/registry) or it can even be included directly in the original package.json.

Just let me know if you have any questions at all and thanks for checking in on this compatibility.

[lehni]

Great, thanks! I thought I did try this already, and the line is already in the current package.json on the develop branch: https://github.com/paperjs/paper.js/blob/develop/package.json#L80

But I must have tested it wrongly, and then assumed it wouldn't fix it.

And even better to hear that this will already be fixed in the upcoming 0.17!

Feel free to close, since you confirmed locally that it will work. Or maybe I should be keep open until 0.17 rolls out?

[guybedford]

An easier method I think is just jspm install github:paperjs/paper.js -o "{format:'amd'}". Let me know if that seems good to you and we can add it into the registry as an override, or alternatively you could add "jspm": { "format": "amd", "jspmNodeConversion": false } to the package.json.

[lehni]

Ok, I think I spoke too quickly. The problem doesn't happen while installing, it happens while loading the library. (I don't personally use JSPM, and haven't looking at this a while ago...). I just installed with the override, and then tested it, and here's what I get:

Uncaught (in promise) Error: (SystemJS) Error: XHR error (404 Not Found) loading http://localhost:9001/@empty/lib/jsdom/living/generated/utils.js(…)
     t @ system.src.js:5123
     p @ system.src.js:5123
     (anonymous function) @ system.src.js:5123

You can test it yourself, using this test-case and the description in the README file:

https://github.com/mrbannon/jspm_npm_test

And here the override I was using:

{
  "browser": {
    "canvas": false,
    "jsdom": false,
    "jsdom/lib/jsdom/living/generated/utils": false,
    "source-map-support": false,
    "./dist/node/window.js": false,
    "./dist/node/extend.js": false
  }
}

[guybedford]

Do try the override above, I think it may be a simpler path forward here.

[guybedford]

(I did test the above case as well myself and saw these issues)

[lehni]

You're right, having them both is the problem:

    "jsdom": false,
    "jsdom/lib/jsdom/living/generated/utils": false,

Alright, so this override works for me:

{
  "browser": {
    "canvas": false,
    "jsdom/lib/jsdom/living/generated/utils": false,
    "source-map-support": false,
    "./dist/node/window.js": false,
    "./dist/node/extend.js": false
  }
}

What's the next step?

[lehni]

Meanwhile I'll test your AMD suggestion now.

[lehni]

Yes, that works too, and so does this:

jspm install npm:paper -o "{format:'amd'}"

[guybedford]

Great, the jspmNodeConversion: true part is necessary when installing from npm to avoid all the other npm stuff coming through (installs from github work differently).

Let me know which route you want to go - maintaining it in the package.json or putting this in the jspm registry. Either is fine!

[lehni]

Great, thanks! Since jspm 0.17 is going to solve this, and we don't have an upcoming release planned but are in the the midst of a larger transition towards 1.0.0, I would prefer the jspm registry so people can already use 0.10.2. I'll let you choose which version is better, both seem to work :)

[guybedford]

@lehni I've added this to the registry now in https://github.com/jspm/registry/commit/cfd3ad6b3fb8e1ce4f5462f613703885faf62d96 and https://github.com/jspm/registry/blob/1516fe96b4b80b6e650a543a2bb851dea2e78397/package-overrides/github/paperjs/paper.js%400.10.2.json. So jspm install paper should now be working.

[lehni]

That's great! I just tested it and I can confirm that it works. Thank you!

[lehni]

Is this fixed to paper.js v0.10.2 now? Is there a way to match a full range, e.g. v0.10.*?

[guybedford]

It will apply for ^0.10.2.

[lehni]

Perfect!