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.

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

Build: Fix regex that removes empty definitions #1569

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@dcorb
Copy link
Contributor

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

This comment has been minimized.

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

This comment has been minimized.

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.

dmethvin added a commit that referenced this pull request Dec 4, 2014

Build: Remove empty define({}) from build output
Fixes gh-1768
Closes gh-1569
(cherry picked from commit 2c1b556)

@dmethvin dmethvin closed this in 2c1b556 Dec 4, 2014

@dmethvin

This comment has been minimized.

Copy link
Member

dmethvin commented Dec 4, 2014

Sorry about the delay!

markelog added a commit that referenced this pull request Nov 10, 2015

@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.