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

Fix for use of AMD define without name/dependencies #49

Closed
wants to merge 1 commit into from

Conversation

elerch
Copy link

@elerch elerch commented Dec 21, 2012

Spin.js 
(https://github.com/fgnass/spin.js/blob/gh-pages/spin.js#L316)

uses define without any dependencies, so deps becomes undefined. This 
usage appears valid based on the AMD definition: 

http://wiki.commonjs.org/wiki/Modules/AsynchronousDefinition

While spin.js' use of define is relatively not useful, it's valid.
This change will accommodate modules that work in this manner.

Spin.js 
(https://github.com/fgnass/spin.js/blob/gh-pages/spin.js#L316)

uses define without any dependencies, so deps becomes undefined. This 
usage appears valid based on the AMD definition: 

http://wiki.commonjs.org/wiki/Modules/AsynchronousDefinition

While spin.js' use of define is relatively not useful, it's valid.
This change will accommodate modules that work in this manner.
@jrburke
Copy link
Member

jrburke commented Jan 1, 2013

Almond only works with built files, files that have all dependencies inlined and given names (string IDs for the first define() call). So the pathway you hit is only hit if the tool used to bundle spin.js with almond failed to insert a string ID for spin's define() call.

Can you give more info on how you generated a built file with almond and spin that resulted in no name insertion for spin? The bug is likely in that process.

Closing this for now, but we can reopen if the built file does end up with the string ID in it for spin's define() call. We should continue discussion in this ticket too, to resolve how this code branch was reached.

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

Successfully merging this pull request may close these issues.

None yet

2 participants