Skip to content

Build: Fix regex that removes empty definitions #1569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Build: Fix regex that removes empty definitions #1569

wants to merge 3 commits into from

Conversation

dcorb
Copy link
Contributor

@dcorb dcorb commented May 1, 2014

Bug

The regex does not match the case define([]); , so it's not removed from the final output.
Reproduce the bug with this command: grunt build:* (Building an empty jQuery)
The output should be an empty jQuery build, with only the wrappers "src/intro.js"and src/outro.js"
but it outputs also these two lines:

      define([]); 
      define("jquery", function(){});

When loading this jquery.js build in the browser, it will give you a "define" is not defined error.
See bug ticket: http://bugs.jquery.com/ticket/15061#comment:2

Fix

This PR fixes the first "define", I'm working to fix the other "define" that shouldn't be there neither.

@dcorb
Copy link
Contributor Author

dcorb commented May 1, 2014

I found the reason for the other "define". Require.js inserts this placeholder.
Setting skipModuleInsertion: true fixes the issue.

From: https://github.com/jrburke/r.js/blob/master/build/example.build.js

    //If skipModuleInsertion is false, then files that do not use define()
    //to define modules will get a define() placeholder inserted for them.
    //Also, require.pause/resume calls will be inserted.
    //Set it to true to avoid this. This is useful if you are building code that
    //does not use require() in the built project or in the JS files, but you
    //still want to use the optimization tool from RequireJS to concatenate modules
    //together.
    skipModuleInsertion: false,

@dmethvin
Copy link
Member

dmethvin commented May 7, 2014

@dcorb, thanks for the fix! Looks like you've signed the CLA so we'll land this in 1.12/2.2 shortly.

@mgol mgol mentioned this pull request Oct 21, 2014
Closed
dmethvin pushed a commit that referenced this pull request Dec 4, 2014
@dcorb @dmethvin
Build: Remove empty define({}) from build output
2138f15 
Fixes gh-1768
Closes gh-1569
(cherry picked from commit 2c1b556)
@dmethvin dmethvin closed this in 2c1b556 Dec 4, 2014
@dmethvin
Copy link
Member

dmethvin commented Dec 4, 2014

Sorry about the delay!

This was referenced May 9, 2015
Closed
Closed
markelog pushed a commit that referenced this pull request Nov 10, 2015
@dcorb @markelog
Build: Remove empty define({}) from build output
0a2f1e4 
Fixes gh-1768
Closes gh-1569
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dcorb @dmethvin