Skip to content
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

Dependencies with RequireJS #14

Closed
TimFletcher opened this issue Aug 19, 2015 · 2 comments
Closed

Dependencies with RequireJS #14

TimFletcher opened this issue Aug 19, 2015 · 2 comments

Comments

@TimFletcher
Copy link

I'm building and embeddable JS widget which has a requirement not to inject any new global variable to the page. I'm using requireJS to do this and importing TimeMe as a module. The import works, but because TimeMe expects ifvisible to be available in the global scope, and it's isn't via RequireJS, it can't initialize properly.

I've made a couple of changes that I'd like to get your opinion on as I'm not that familiar with AMD modules and requireJS.

I've added ifvisible as a dependent module in define and set it as a property (which I don't think I like):

if (typeof define === "function" && define.amd) {
    define(['ifvisible'], function(ifvisible) {
      TimeMe.ifvisible = ifvisible;
      return TimeMe;
    });
} else {
    window.TimeMe = TimeMe;
}

and then detected this.ifvisible in getIfVisibleHandle to set the handle correctly.

getIfVisibleHandle: function(){
    if (typeof ifvisible === 'object') {
        return ifvisible;
    else if (typeof this.ifvisible === 'object') {
        return this.ifvisible;
    } else {
        if (typeof console !== "undefined") {
            console.log("Required dependency (ifvisible.js) not found.  Make sure it has been included.");
        }
        throw {
            name: "MissingDependencyException",
            message: "Required dependency (ifvisible.js) not found.  Make sure it has been included."
        };
    }
},

I think ideally i'd rewrite the class to have a constructor function and pass in the dependency but it's a lot of changes. Interested to hear your thoughts on this!

@jasonzissman
Copy link
Owner

Hi Tim! Thanks for reaching out. I unfortunately have not used Require.js
or AMD modules extensively myself. Both of the code snippets that you are
showing look fine to me, however. I actually like the idea of setting the
ifvisible handler directly against the TimeMe object. I'll do a refactor
later, I think, to simply refer back to an ifvisible property set directly
on the TimeMe objec instead of executing the getIfVisibleHandle logic every
single time I need ifvisible.

I assume you'll be making a pull request. Let me know if I can help with
anything.

On Tue, Aug 18, 2015 at 10:46 PM, Tim Fletcher notifications@github.com
wrote:

I'm building and embeddable JS widget which has a requirement not to
inject any new global variable to the page. I'm using requireJS to do this
and importing TimeMe as a module. The import works, but because TimeMe
expects ifvisible to be available in the global scope, and it's isn't via
RequireJS, it can't initialize properly.

I've made a couple of changes that I'd like to get your opinion on as I'm
not that familiar with AMD modules and requireJS.

I've added ifvisible as a dependent module in define and set it as a
property (which I don't think I like):

if (typeof define === "function" && define.amd) {
define(['ifvisible'], function(ifvisible) {
TimeMe.ifvisible = ifvisible;
return TimeMe;
});
} else {
window.TimeMe = TimeMe;
}

and then detected this.ifvisible in getIfVisibleHandle to set the handle
correctly.

getIfVisibleHandle: function(){
if (typeof ifvisible === 'object') {
return ifvisible;
else if (typeof this.ifvisible === 'object') {
return this.ifvisible;
} else {
if (typeof console !== "undefined") {
console.log("Required dependency (ifvisible.js) not found. Make sure it has been included.");
}
throw {
name: "MissingDependencyException",
message: "Required dependency (ifvisible.js) not found. Make sure it has been included."
};
}
},

I think ideally i'd rewrite the class to have a constructor function and
pass in the dependency but it's a lot of changes. Interested to hear your
thoughts on this!


Reply to this email directly or view it on GitHub
#14.

  • Jason Christopher Zissman

@TimFletcher
Copy link
Author

Sure, I can submit a pull request. Will do shortly :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants