reflux / reflux-core

A simple core library for uni-directional dataflow application architecture inspired by ReactJS Flux
BSD 3-Clause "New" or "Revised" License
48 stars 19 forks source link

checkEnv causes CSP error in Firefox 40 #6

Closed eanakashima closed 9 years ago

eanakashima commented 9 years ago

Looks like the checkEnv function in utils.js does an eval to see if certain functions exist in the current environment, and if you have restrictive CSP settings that don't allow eval, this will throw an error in Firefox 40 (even with the try block wrapping it):

screen shot 2015-09-01 at 2 27 13 pm

I thought about opening a PR with a fix but I'm not quite sure what a good solution would be. In the browser something like this would work:

function checkEnv(target) {
    var flag = false;

    if (window[target]) { flag = true; }

    environment[callbackName(target, "has")] = flag;
}

But that doesn't play nice when there's no window. Any suggestions?

LongLiveCHIEF commented 9 years ago

Reflux-core is not written for the browser, it's a node.js module. You'll need to transpile it with something like browserify and babel if you want to use reflux-core in the browser.

Alternatively, you can use reflux.js instead, which has shims around the library, and a few extra functions.

If this still errors out once browser-ified, or if it errors out in reflux.js

Side note... these functions should still be refactored/removed as needed, based on the reflux-core migration to node and server-side.

There's going to be a bit of work associated with this one. One of the core decisions regarding the development of reflux thus far has been to limit the dependencies that are included with reflux.

These utility functions are there so you can provide your own shims for promises and browser compatibility, and then have those available behind a common interface inside reflux.js, and aren't needed in reflux-core.

eanakashima commented 9 years ago

@LongLiveCHIEF Thanks for the context.

We're using reflux.js but I opened the issue here when I saw that the error was coming from reflux-core code. I can raise the issue in reflux.js instead.

spoike commented 9 years ago

@eanakashima Yes, sorry about that. You'll need to run older version of refluxjs (0.2.11 I believe if the CHANGELOG is right) until I remove the eval check and replace the current promise methods with the simpler deferWith solution (in #8).

eanakashima commented 9 years ago

@spoike good to know, thank you!