Build: Fix the regex parsing AMD var-modules #4389
Conversation
There was a problem hiding this comment.
The current regex works for me, even when the module is named returnTrue. https://jsbin.com/kemuvaj/edit?html,js,console,output
This change doesn't actually seem to match.
Note the ? is there to be non-greedy, meaning it stops on the first occurrence of "return", not going all the way to "returnTrue".
Edit: I realized the current regex would break if another var module included returnTrue as a dependency. However, the first argument to define is not a string, but an array, and it's an optional argument. I suggest the following instead:
/define\((?:\s*\[.*\]\s*)?[\w\W]*?return/
jsbin above updated.
|
@timmywil The value of the define( 'var/returnTrue',[],function() {
"use strict";
return function returnTrue() {
return true;
};
} );After the replacement it is: var returnTrue =True',[],function() {
"use strict";
return function returnTrue() {
return true;
};See the updated bin: https://jsbin.com/mufamok/1/edit?html,js,console,output |
|
This PR is a result of my experiments around #4387. In commit mgol@9a9c3f0 I had to change the regexp because it was mangling my newly-separated |
|
Ah, I forgot it inserts the name. Ok. |
| contents = contents | ||
| .replace( | ||
| /define\([\w\W]*?return/, | ||
| /define\(\s*(["'])[\w\W]*?\1[\w\W]*?return/, |
There was a problem hiding this comment.
Simpler suggestion: https://jsbin.com/wozinogigo/1/edit?js,console,output
| /define\(\s*(["'])[\w\W]*?\1[\w\W]*?return/, | |
| /define\([\w\W]*?\breturn\b/, |
There was a problem hiding this comment.
Technically, there could be a directory named return. Not the best idea, perhaps, but my regex should work for all of those weird cases.
The previous regex caused the final jQuery binary to have syntax errors for
var-modules with names starting with "return". For example, the following module
wouldn't work when the file is named `returnTrue.js`:
```js
define( function() {
"use strict";
return function returnTrue() {
return true;
};
} );
```
800ae7d to
1dcad4f
Compare
The previous regex caused the final jQuery binary to have syntax errors for
var-modules with names starting with "return". For example, the following module
wouldn't work when the file is named `returnTrue.js`:
```js
define( function() {
"use strict";
return function returnTrue() {
return true;
};
} );
```
Closes gh-4389
(cherry picked from commit 9ec09c3)
|
Cherry-picked to |
Summary
Fix the regex parsing AMD var-modules.
The previous regex caused the final jQuery binary to have syntax errors for
var-modules with names starting with "return". For example, the following module
wouldn't work when the file is named
returnTrue.js:Checklist
New tests have been added to show the fix or feature worksN/AIf needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.comN/A