Opened 10 years ago

Closed 10 years ago

#15509 closed defect (wontfix)

Naming collision with dojoConfig.packages when name is "module"

Reported by: mmaxwell Owned by: Rawld Gill
Priority: undecided Milestone: tbd
Component: Loader Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description

When using the variable dojoConfig prior to including Dojo, if a package name is set as "module", it appears as though the name collides with something native to the library.

Attaching screenshot of error as well as zipped test case.

Attachments (3)

error.png (112.1 KB) - added by mmaxwell 10 years ago.
Screenshot of error.
test.zip (842 bytes) - added by mmaxwell 10 years ago.
Zipped test case.
15509.patch (931 bytes) - added by Rawld Gill 10 years ago.
patch allows package names require, exports, and module

Download all attachments as: .zip

Change History (8)

Changed 10 years ago by mmaxwell

Attachment: error.png added

Screenshot of error.

Changed 10 years ago by mmaxwell

Attachment: test.zip added

Zipped test case.

comment:1 Changed 10 years ago by bill

Component: GeneralLoader
Owner: set to Rawld Gill

comment:2 Changed 10 years ago by ben hockey

the names "require", "exports" and "module" have special meaning and are reserved by the AMD loader. you'll need to use another name. while you could argue that this is technically a regression, i'd say this will be closed as wontfix but i'll leave it open for rawld to make any further comments.

comment:3 Changed 10 years ago by mmaxwell

Makes sense. I've already changed the name in my code. Is this documented anywhere? Is it possible to detect an override attempt and throw?

comment:4 Changed 10 years ago by ben hockey

the documentation is here http://dojotoolkit.org/reference-guide/1.7/loader/amd.html#commonjs-require-exports-and-module - admittedly if you don't know what you're looking for you might not find it but i don't know how to improve that when there's such a lot of documentation.

as for detecting an override, it could possibly be done but i'm not sure if it's worth the bytes it would take. i would tend towards not adding it but i'll leave that decision to rawld.

comment:5 Changed 10 years ago by Rawld Gill

Resolution: wontfix
Status: newclosed

@neonstalwart is correct.

Technically, the AMD spec doesn't say that the packages require, exports, and module are reserved, but I can't imagine defining packages with those names, and, although I fixed this, I couldn't bring myself to committing the cruft. I've attached a patch below if you absolutely need it.

Changed 10 years ago by Rawld Gill

Attachment: 15509.patch added

patch allows package names require, exports, and module

Note: See TracTickets for help on using tickets.