jspm / registry

The jspm registry and package.json override service
https://jspm.io
229 stars 256 forks source link

Update moment@2.0.0.json #946

Closed fluffywaffles closed 7 years ago

fluffywaffles commented 7 years ago

Fixes the "AMD module did not define" error that sometimes occurs when trying to import moment.

Recreate issue:

$ node -e "require('jspm').import('moment').then(console.log.bind(console)).catch(console.log.bind(console))"

{ (SystemJS) AMD module /media/Data/dev/work/encurate/ecm-api/functions/authorizer/jspm_packages/npm/moment@2.14.1/moment.js did not define
        TypeError: AMD module /media/Data/dev/work/encurate/ecm-api/functions/authorizer/jspm_packages/npm/moment@2.14.1/moment.js did not define
        Error loading /media/Data/dev/work/encurate/ecm-api/functions/authorizer/jspm_packages/npm/moment@2.14.1/moment.js
  originalErr:
   TypeError: AMD module file:///media/Data/dev/work/encurate/ecm-api/functions/authorizer/jspm_packages/npm/moment@2.14.1/moment.js did not define
       at SystemJSNodeLoader.<anonymous> (/media/Data/dev/work/encurate/ecm-api/functions/authorizer/node_modules/systemjs/dist/system.src.js:4347:19)
       at SystemJSNodeLoader.instantiate (/media/Data/dev/work/encurate/ecm-api/functions/authorizer/node_modules/systemjs/dist/system.src.js:4619:28)
       at /media/Data/dev/work/encurate/ecm-api/functions/authorizer/node_modules/systemjs/dist/system.src.js:433:33 }

Once the format override is added things work. Or appear to at least.

Strangely, the error only occurs when trying to import from the command line using node -e. If I load up the node REPL and run the exact same code, no error occurs.

fluffywaffles commented 7 years ago

Weird. I'm really not sure why this error happens, or why it only happens when using node -e. See edited PR - sorry for the confusion, @guybedford !

I don't think this actually fixes it anyway. The result of console.loging the module that comes from jspm.import('moment') is an empty object...

fluffywaffles commented 7 years ago

I am giving up on trying to run a node script that calls jspm.import from the command line for now, seeing as this error only shows up then.

fluffywaffles commented 7 years ago

I don't feel comfortable making a PR for it, but setting "format: 'cjs'" fixes this for me. That said, I'm only running it in Node, so I'm not sure that just saying "cjs" works in every case. I guess you might just need a different override on a per-case basis, which is kinda gross.

guybedford commented 7 years ago

Which version of jspm are you using? Have you tried jspm 0.17?

fluffywaffles commented 7 years ago

I am using JSPM v0.17.0-beta.23 and npm 3.10.5. Like I said, I can get it to work by explicitly setting format to "cjs," but if I don't have any format override it fails and if I set the format to "umd" (which, as you said, is an invalid option) then I just get back "{}" instead of the module.

On Tue, Aug 2, 2016 at 6:27 AM Guy Bedford notifications@github.com wrote:

Which version of jspm are you using? Have you tried jspm 0.17?

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/jspm/registry/pull/946#issuecomment-236876996, or mute the thread https://github.com/notifications/unsubscribe-auth/AA2LgnzxtqjIf_3p-SZZQnc36_bT4arNks5qbymZgaJpZM4JZ5On .

guybedford commented 7 years ago

@skorlir this definitely works fine for me in jspm 0.17 with the following:

test.js

import moment from 'moment';
console.log(moment);
jspm install npm:moment
jspm build test.js --node
node build.js

Equally the following works fine too in Node:

var jspm = require('jspm');
jspm.import('moment').then(console.log.bind(console));

Perhaps try jspm install npm:moment -fo to force clear the cache and all local overrides.

fluffywaffles commented 7 years ago

@guybedford note that, as I mentioned in the (edited version) of the original PR, I only got an error when trying to run the script inline with node -e '...'

I'm not sure why this would make a difference, but it seemed to.

I am still able to recreate with

$ node -e "require('jspm').import('moment').then(console.log.bind(console)).catch(console.log.bind(console))"

When I jspm i npm:moment it fails with an error. When I jspm i npm:moment -o "{format: 'cjs'}" it succeeds.

guybedford commented 7 years ago

@skorlir this sounds like a node -e specific issue with executing AMD then I think.

fluffywaffles commented 7 years ago

If I npm i moment and then node -e "console.log(require('moment'))" it works.

This is pretty strange.

fluffywaffles commented 7 years ago

@guybedford moment is distributed as UMD, though, so I'm not sure why it would ever try to register an AMD module if it's being used in Node. But for some reason when importing it with JSPM via node -e on the command line, unless the format override is in place, it does.

guybedford commented 7 years ago

@skorlir supporting node -e execution for SystemJS is not a high priority right now, but I've created https://github.com/systemjs/systemjs/issues/1399 to investigate this further.