Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13285 closed defect (fixed)

unterminated regular expression literal with commented dojo.provide

Reported by: Kate Glickman Owned by: Rawld Gill
Priority: high Milestone: 1.7
Component: Loader Version: 1.7.0b1
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by dante)

I have a commented out dojo.provide line in between to uncommented out dojo.provide lines, like this:

dojo.provide("myclassA");
//dojo.provide("myclassB");
dojo.provide("myclassC");

Firefox gives me the error message:

unterminated regular expression literal /require.modules*myclassA?.provide("myClassB");

If I remove the commented out line, it works fine.

Change History (14)

comment:1 Changed 8 years ago by Kate Glickman

I should have done preview. The 3 dojo.provide lines are not actually on a single line like this posting shows. There is a line feed after each ;.

comment:2 Changed 8 years ago by dante

Description: modified (diff)

comment:3 Changed 8 years ago by Douglas Hays

Component: GeneralCore
Owner: set to Rawld Gill

comment:4 Changed 8 years ago by Douglas Hays

hookProvides may also need to handle block comments, and possibly even string constants

comment:5 Changed 8 years ago by bill

Component: CoreLoader

comment:6 Changed 8 years ago by dante

just to note: you should only ever have one provide() call in a module (from 0.9+) ... the error is from a commented out line of code and shouldn't happen, just worth pointing out multiple provides are not technically supported. If you need to get an object from a string (the only other real thing dojo.provide() does is expand "a.b.c" to a.b.c = {}) you should use getObject

dojo.getObject("a.b.c", true); 

comment:7 Changed 8 years ago by Rawld Gill

In legacy synchronous mode, the machinery that is hooking dojo.provides in order to publish module values back to the AMD loader module space failed to remove comments before scanning for dojo.provide applications. Machinery that is doing on-the-fly AMD conversion in xdomain and async legacy modes is correctly removing comments so necessary functions are already in the loader.

Also noted while duplicating/testing this problem...the v1.6- comment removal process (which was copied for 1.7) is weak: string and regular expressions contained in the text being transformed can break the comment removal code resulting in the supposed cleaned text being essentially garbage. Also, strings and regexs that remain in the cleaned text can break other regex operations further down in the transform process. This is not entirely fixable without moving to a proper parser, but the situation can be improved by implementing more-robust regexs.

comment:8 Changed 8 years ago by Rawld Gill

Resolution: fixed
Status: newclosed

(In [25584]) improved comment removal regexs used to process legacy modules; added tests; fixes #13285; !strict

comment:9 Changed 8 years ago by Rawld Gill

(In [25595]) rolled back [25584] since improvement just broke, albeit less often, in a different way; refs #13285; !strict

comment:10 Changed 8 years ago by Rawld Gill

Incorrectly thought this was a problem with comment removal which motivated [25584], an ill-fated attempt to improve the v1.6- method of comment removal.

v1.6- attempts comment removal with a regex which will fail miserably is some cases. The particular regex used in v1.6- can be improved, but alternative regex designs that result in fewer failures will still fail in many cases. The only solution that is 100% correct is to parse the code and that seems overkill for this backcompat/unbuilt-xdomain problem. In the end, since it's been this way for a while, we won't change it.

[25595] changes the way the hint is added to eval'd code in the loader since the function constructor expression was not adding the hint correctly. This may have been the actual cause of the problem reported, but was unable to duplicate.

comment:11 in reply to:  1 Changed 8 years ago by Rawld Gill

Replying to kglickman:

I should have done preview. The 3 dojo.provide lines are not actually on a single line like this posting shows. There is a line feed after each ;.

I was not able to duplicate. Could you attach the entire module so I can take a look to make sure we got this one fixed?

Thanks!

comment:12 Changed 8 years ago by Kate Glickman

I didn't write this module and when I reported this, I didn't look farther than the first few lines of the file. I'm new to dojo and now that I'm looking at the rest, I'm not even sure the syntax is valid.

dojo.provide("my.namespace.util.url"); dojo.provide("my.namespace.util.url.UrlStruct?"); dojo.provide("my.namespace.util.url.ProxyUrlHelper?"); dojo.require("my.namespace.util.proxy"); (function(){

var url = my.namespace.util.url;

/* dojo.declare("my.namespace.util.url.UrlStruct?", null, { });*/

url.ProxyUrlHelper? = function() {}; /* dojo.declare("my.namespace.util.url.ProxyUrlHelper?", null, { });*/

})();

comment:13 Changed 8 years ago by Rawld Gill

(In [25636]) improved backcompat dojo.provide processing to account for sources without newline on last line; refs #13285; !strict

comment:14 Changed 8 years ago by bill

Milestone: tbd1.7
Note: See TracTickets for help on using tickets.