Opened 7 years ago

Closed 7 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 7 years ago.
Screenshot of error.
test.zip (842 bytes) - added by mmaxwell 7 years ago.
Zipped test case.
15509.patch (931 bytes) - added by Rawld Gill 7 years ago.
patch allows package names require, exports, and module

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by mmaxwell

Attachment: error.png added

Screenshot of error.

Changed 7 years ago by mmaxwell

Attachment: test.zip added

Zipped test case.

comment:1 Changed 7 years ago by bill

Component: GeneralLoader
Owner: set to Rawld Gill

comment:2 Changed 7 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 7 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 7 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 7 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 7 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.