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

require.js conflict #112

Closed
nariman-haghighi opened this issue Jul 22, 2014 · 39 comments · Fixed by #113
Closed

require.js conflict #112

nariman-haghighi opened this issue Jul 22, 2014 · 39 comments · Fixed by #113

Comments

@nariman-haghighi
Copy link

nariman-haghighi commented Jul 22, 2014

UPDATE: Read the resolution below


The issue is reproduced here:
http://www.candid.io/joefresh.html

The JS error is referencing this doc: http://requirejs.org/docs/errors.html#mismatch

We've done a number of integrations with our script (http://api.getcandid.com/scripts/widget.js) which references keen 3.0.4... this is the first conflict we've seen.

Any ideas would be appreciated.

@wetzler
Copy link

wetzler commented Jul 22, 2014

Thanks for filing Nariman - I believe this is related to #99

@dustinlarimer
Copy link
Contributor

@nariman-haghighi yep, we're debugging this in #99 and have some ideas here: https://github.com/keenlabs/keen-js/blob/d2ae28359b420a427c00534edfe0693f56f03c4a/test/examples/script-loaders/require/app.js#L4-L7

Care to weigh in? Would love your perspective on an effective pattern here!

@dustinlarimer
Copy link
Contributor

Check out #113.

@nariman-haghighi
Copy link
Author

@dustinlarimer Does the CDN copy have this change now?

@wetzler
Copy link

wetzler commented Jul 22, 2014

Per Nariman, the same bug is still present after trying this newest version of the library.

@dustinlarimer
Copy link
Contributor

I'll post and update when the latest distribution has this fix – thanks!

On Tue, Jul 22, 2014 at 12:56 PM, Michelle Wetzler <notifications@github.com

wrote:

Per Nariman, the same bug is still present after trying this newest
version of the library.


Reply to this email directly or view it on GitHub
#112 (comment).

@nariman-haghighi
Copy link
Author

Thanks Dusitn. FWIW, we're trying the track-only build and that has the same issue, so there's no viable workaround at the moment. Do you suggest we deploy the full build from master to our own CDN and try that?

@dustinlarimer
Copy link
Contributor

I'll have this finished and deployed to our CDN later today – sorry for the
inconvenience!

On Tue, Jul 22, 2014 at 1:19 PM, Nariman Haghighi notifications@github.com
wrote:

Thanks Dusitn. FWIW, we're trying the track-only build and that has the
same issue, so there's no viable workaround at the moment. Do you suggest
we deploy the full build from master to our own CDN and try that?


Reply to this email directly or view it on GitHub
#112 (comment).

@dustinlarimer
Copy link
Contributor

@nariman-haghighi v3.0.5 is in our CDN, and I'll publish a tagged release shortly:
http://d26b395fwzu5fz.cloudfront.net/3.0.5/keen.min.js

@nariman-haghighi
Copy link
Author

Running 3.0.5 tracking only, the error is still visible here: http://www.candid.io/joefresh.html

@dustinlarimer
Copy link
Contributor

@nariman-haghighi it looks like there is another anonymous define call in that script, which appears to fit the diagnosis from the requireJS mismatch link that you shared. From what i've gathered fighting with requirejs over the past week, these types of scenarios need to be remedied with the Optimizer, which fixes paths/modules IDs for modules existing on the same file.

Can you share a bit about your implementation method for the keen library? I'd like to understand better why it's being loaded the way it is.

@dustinlarimer dustinlarimer reopened this Jul 22, 2014
@nariman-haghighi
Copy link
Author

@dustinlarimer are you suggesting that we can work around the error by loading it differently? we are following the async pattern as published here to bring in your script through our widget.js: https://github.com/keenlabs/keen-js/wiki/Installation

@nariman-haghighi
Copy link
Author

We don't control the require.js call on the parent page... we assume keen.io usage is safe "in the wild" when we import it through our widget.js.

@dustinlarimer
Copy link
Contributor

Totally understand, it just appears that the error is related to using multiple anonymous module definitions in the same file.

The conflict there is that the RequireJS design pattern also insists that modules are defined anonymously, in their own file. 

I'm doing some research on my end to better understand what we can do to avoid this issue without breaking expectations for other implementation models.

@dustinlarimer
Copy link
Contributor

@nariman-haghighi this issue seems to be capturing the same issue we're seeing here:
umdjs/umd#11 (comment)

@nariman-haghighi
Copy link
Author

We can't control where our widget.js (and hence keen.js) is loaded relative to the requirejs on the parent page. I guess that leaves us with one option. We are open to a conditional import of your script that uses require/define if that works? Just let us know the pattern we can try.

@dustinlarimer
Copy link
Contributor

Also, it appears neither the keen lib nor your widgets.js file are loaded with RequireJS.. Is that correct?

On Tue, Jul 22, 2014 at 3:59 PM, Nariman Haghighi
notifications@github.com wrote:

We don't control the require.js call on the parent page... we assume keen.io usage is safe "in the wild" when we import it through our widget.js.

Reply to this email directly or view it on GitHub:
#112 (comment)

@nariman-haghighi
Copy link
Author

Correct.

@dustinlarimer
Copy link
Contributor

@nariman-haghighi there is an anonymous define() call in your widgets.js file.. would it be possible to give it an explicit id, eg: define("myModule", function(){ .. });?

@nariman-haghighi
Copy link
Author

Are you referring to define(function () { return A })?

It's part of https://github.com/bestiejs/json3

@dustinlarimer
Copy link
Contributor

Ok, gotcha – here's a couple of approaches that may work here:

  • use requirejs to actually load our lib (in place of our async shim) + json3, then append the modules to the global context.
  • fork our repo and create a custom build that strips the requirejs wrapper out entirely.
  • run all of this through the j.js optimizer to set explicit module IDs .

Do any of these sound doable?

@nariman-haghighi
Copy link
Author

What about ensuring the tracking only build doesn't depend on requirejs? Shouldn't it be safe anywhere? Our widget.js is ~10KB, adding the 15KB requirejs to load other libraries doesn't add up.

@nariman-haghighi
Copy link
Author

With JSON3 removed, there is still a conflict between keen and the parent page:

http://www.candid.io/joefresh.html

@dustinlarimer
Copy link
Contributor

The wrapper is only designed to define itself as a module when requirejs is present, just as JSON3 does here. The conflict seems to be coming from the fact that our library is being included alongside, though not used with requirejs.

I'm looking for better patterns to follow, since we can't control when, where or how we're loaded into a page.

@dustinlarimer
Copy link
Contributor

@nariman-haghighi would you mind giving this a try? Sorry for all the back-and-forth here, I really appreciate your help in ironing this out!
http://d26b395fwzu5fz.cloudfront.net/staging/keen-tracker.min.js

@nariman-haghighi
Copy link
Author

Works!

@dustinlarimer
Copy link
Contributor

Excellent, I'll get it pushed out and let you know when a versioned release is available!

On Wed, Jul 23, 2014 at 2:58 PM, Nariman Haghighi
notifications@github.com wrote:

Works!

Reply to this email directly or view it on GitHub:
#112 (comment)

@nariman-haghighi
Copy link
Author

Great, please keep that version up in the meantime.

@dustinlarimer
Copy link
Contributor

#117

@dustinlarimer
Copy link
Contributor

@nariman-haghighi v3.0.5 has been updated.. You may need to tack on a cache-buster parameter to bring in the latest:
http://d26b395fwzu5fz.cloudfront.net/3.0.5/keen.js

@elliottshort
Copy link

This has resurfaced in v3.4.0-rc.

@dustinlarimer
Copy link
Contributor

@elliottshort thanks for reporting - would you mind sharing a bit more about your setup? Which version of RequireJS is causing the issue, and how are you loading/configuring the library? Thanks for your help!

@agrapsas
Copy link

agrapsas commented Mar 3, 2016

Also affecting my project with 3.4.0

require.config( {
    'baseUrl' : 'app',
    'paths' : {
        'keen' : "../lib/keen-js/dist/keen"
    },
   ...
} );
'use strict';

require( [ "angularAMD", "keen" ], function( angularAmd, Keen )
{

    angularAmd.service( 'AnalyticsService', function()
    {
        return Keen;
    } );

} );

I immediately load it into my AngularAMD project. If I refresh without clearing cache, everything then loads fine. If I clear cache, it dies.

Edit: For now, I am ditching the library because I only need a small set of functions. Instead, I am just directly hitting your api.

@dustinlarimer
Copy link
Contributor

@centaur2048 sorry to hear about this– which version of requirejs are you using?

@dustinlarimer dustinlarimer reopened this Mar 3, 2016
@agrapsas
Copy link

agrapsas commented Mar 3, 2016

Requirejs version: 2.1.16

@agrapsas
Copy link

No resolution.
I've tried loading this on the page using script tags and directly using RequireJs configuration. Neither has worked.

I am at the point where I want to begin using your chart libraries; so, would be nice to resolve this. I just pulled 3.4.1-RC4 and the issue still persists.

Also using RequireJs 2.2.0 now.

@dustinlarimer
Copy link
Contributor

@centaur2048 Google Charts shipped a UMD/RequireJS bug that has been causing intermittent and difficult-to-debug crashes, so we just recently shipped a version of keen-js that omits GC entirely. Please give this file a try and let me know how it goes:
https://d26b395fwzu5fz.cloudfront.net/3.4.1-rc4/keen-c3.js

@agrapsas
Copy link

Excellent! Tested with both script tag and using RequireJs and the issue appears to be resolved.

As this version omits GC, does it have any limitations? Will I not be able to render charts with it?

@dustinlarimer
Copy link
Contributor

Closing now– this issue has stood open for a long time as a point of reference for others struggling with the issue, but now our v3 docs should properly highlight the issue. Thanks again @nariman-haghighi @agrapsas for the help here!

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

Successfully merging a pull request may close this issue.

5 participants