Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#17438 closed defect (fixed)

[patch] builder fails to resolve modules that use dojo/has with other loader plugins

Reported by: chuckd Owned by: Rawld Gill
Priority: high Milestone: 1.9.3
Component: BuildSystem Version: 1.9.1
Keywords: Cc: cjolif
Blocked By: Blocking:

Description

For example:

dojo/has!foo?dojo/text!foo.txt:dojo/text!bar.txt

If foo is defined to be 1 in staticHasFeatures, then the builder will output the following error:

error(302) Missing dojo/has plugin resource that was resolved at build-time. plugin resource id: dojo/has!foo?dojo/text!foo.txt:dojo/text!bar.txt; resolved plugin resource id: dojo/text!bar.txt; reference module id: test/testBuilder.

If dojo/text!bar.txt is specified instead, then the text module is correctly resolved by the builder.

Attachments (2)

has.js.patch (1.6 KB) - added by chuckd 6 years ago.
Fix for ticket #17438
depsScan.js.patch (1.0 KB) - added by chuckd 6 years ago.

Download all attachments as: .zip

Change History (18)

Changed 6 years ago by chuckd

Attachment: has.js.patch added

Fix for ticket #17438

comment:1 Changed 6 years ago by chuckd

I've attached a fix for this problem. Note that the function getAmdModule is copied verbatim from depsScan.js. It would make sense to add this function to the bc so that it can be called by depsScan and the has plugin (as well as anyone else that might need it), but I didn't do that.

Changed 6 years ago by chuckd

Attachment: depsScan.js.patch added

comment:2 Changed 6 years ago by chuckd

Added patch for depsScan.js. The changes to has.js can result in nested arrays of modules returned by getAmdModules(), so the array detection logic in depsScan.js was insufficient.

comment:3 Changed 6 years ago by bill

Chuckd, please submit patches as documented in https://github.com/dojo/util/blob/master/CONTRIBUTING.md, thanks.

Last edited 6 years ago by bill (previous) (diff)

comment:4 Changed 6 years ago by bill

Summary: builder fails to resolve modules that use dojo/has with other loader plugins[patch] builder fails to resolve modules that use dojo/has with other loader plugins

comment:5 Changed 6 years ago by bill

Also, looks like you haven't filed a CLA, so be sure to do that too, thanks!

comment:6 Changed 6 years ago by chuckd

OK, I've issued a pull request for this fix: https://github.com/dojo/util/pull/7 CLA is on the way.

comment:7 Changed 6 years ago by Adam Peller

IBM CCLA will suffice.

comment:8 Changed 6 years ago by chuckd

Could someone please merge this fix into trunk? Thanks

comment:9 Changed 6 years ago by bill

Owner: set to Rawld Gill
Status: newassigned

comment:10 Changed 6 years ago by cjolif

Cc: cjolif added

Someone else reported that one to me. Looks like it would be time to get that one merged.

comment:11 Changed 6 years ago by cjolif

Priority: undecidedhigh

comment:12 Changed 6 years ago by cjolif

Milestone: tbd1.9.3

comment:13 Changed 6 years ago by cjolif

Resolution: fixed
Status: assignedclosed

comment:14 Changed 6 years ago by Colin Snover

Milestone: 1.9.31.10

Not sure why this was set milestone 1.9.3, the patch was only applied against master. I assume you *want* it to be in earlier versions, but you do have to actually backport for that :)

comment:15 Changed 6 years ago by Colin Snover

Milestone: 1.101.9.3

Fixed in 94a0d235fee5303b904c1b2f90d5b3872b92bedd (master) Fixed in 421446fdcb9f63b80b25655c6a634d30686bd401 (1.9)

Is this a 1.8 bug too?

Last edited 6 years ago by Colin Snover (previous) (diff)

comment:16 in reply to:  14 Changed 6 years ago by cjolif

Replying to csnover:

Not sure why this was set milestone 1.9.3, the patch was only applied against master. I assume you *want* it to be in earlier versions, but you do have to actually backport for that :)

Right. Double checking I've pushed that in my fork 1.9 branch but not in dojo/util. I've probably been interupted in the middle of the process and forgot about it later... Thanks a lot for noticing that.

Is this a 1.8 bug too?

I have not tested with 1.8 but I suspect it is.

Note: See TracTickets for help on using tickets.