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

enforceDefine triggers error on main file #298

Closed
cluster-bucket opened this Issue May 31, 2012 · 8 comments

Comments

Projects
None yet
3 participants
@cluster-bucket
Copy link

cluster-bucket commented May 31, 2012

Hey James, just upgraded to Require 2.0. Really nice work. I wanted to report a bug relating to enforceDefine, here's the repro using the require-jquery sample project (also reproducible with the non-require-jquery version, loading jquery as a dep):

// main.js
requirejs.config({
  enforceDefine: true,
  shim: {
    'alpha': ['jquery'],
    'beta': ['jquery']
  }
});

// Change to `require` to `define` and the error will go away
require(["jquery", "alpha", "beta"], function($) {
    //the jquery.alpha.js and jquery.beta.js plugins have been loaded.
    $(function() {
        $('body').alpha().beta();
    });
});

Steps

  1. Set enforceDefine: true
  2. require() some modules as normal

Expected
All modules loaded without errors

Result
All modules loaded, BUT error No define call for main http://requirejs.org/docs/errors.html#nodefine displayed in console

Workaround
Change require to define.

I've repro'd the bug in Chrome/Firefox latest. Let me know if you need anything else or would like me to email you the files.

@jrburke

This comment has been minimized.

Copy link
Member

jrburke commented May 31, 2012

Ah, right, I should probably exclude the data-main from the define enforcement. I'll mark this for 2.0.1 which I hope to do before the weekend.

@cluster-bucket

This comment has been minimized.

Copy link

cluster-bucket commented May 31, 2012

@jrburke Thanks!

@jrburke

This comment has been minimized.

Copy link
Member

jrburke commented Jun 1, 2012

Hmm, thinking more about this, perhaps it is best to document this behavior -- that if enforceDefine is on, then your data-main module needs to use define() too, and then if building with almond, you will need the inserRequire: ['main']) in the build config.

Otherwise, if someone did want to detect a failure in the data-main, then there would not be a way to do it. And the enforceDefine is doing its job, enforcing all modules use define or have a shim config.

How does that sound?

@cluster-bucket

This comment has been minimized.

Copy link

cluster-bucket commented Jun 1, 2012

Sounds good but what if you're not building with almond? As a note, I am using it to do checking for failures.

@jrburke

This comment has been minimized.

Copy link
Member

jrburke commented Jun 1, 2012

If you are not building with almond, then changing that one require() call to define() is enough, since data-main acts as the first require call. In an almond build, there is not a data-main processing so it needs a require inserted if there is not already one there.

@jrburke jrburke closed this in 94e8c9c Jun 1, 2012

@jrburke

This comment has been minimized.

Copy link
Member

jrburke commented Jun 1, 2012

I added a note to the IE load error section about enforceDefine:
http://requirejs.org/docs/api.html#ieloadfail

and updated the Upgrade doc with the same note.

@cluster-bucket

This comment has been minimized.

Copy link

cluster-bucket commented Jun 1, 2012

@jrburke Looks great. Thanks again! Can't wait to dig into almond a bit.

@restarian

This comment has been minimized.

Copy link

restarian commented May 20, 2017

It seems like enforceDefine should exclude any config passed in with a require call. The main file below keeps the config seperate but enforceDefine still errors.
main.js
`
require(["require_config"], function(config) {

require(["dom_ready"], function() {

    require(["two", "three"], function(two, three) {

        two.hi()

        three.hi()

    })

})

})
`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment