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 is not defined on browser #32

Closed
igrep opened this issue Nov 19, 2015 · 15 comments
Closed

require is not defined on browser #32

igrep opened this issue Nov 19, 2015 · 15 comments

Comments

@igrep
Copy link

igrep commented Nov 19, 2015

Since 3.0.0, executing test on a browser always causes an error that

require is not defined


Open the developer console for more details.

Is running on browser disabled from v3.0.0?

@FranklinChen
Copy link

I just ran into this also. I think it's some kind of compilation problem?

@deadfoxygrandpa
Copy link
Owner

It shouldn't be disabled. It worked on my computer. I'll look into this right away.

@jackfranklin
Copy link

Just to add, I've just upgraded from 0.15 to 0.16 and the latest corresponding version of elm-test and I'm also seeing this error. Have had a quick look into this but haven't found anything yet.

@deadfoxygrandpa
Copy link
Owner

OK, I've tracked down the problem. It's due to importing the console runner by default now, which evaluates elm-console on page load, which contains code that runs in node but crashes the browser.

I can add some runtime checks in elm-console that disable the node-specific features if run in the browser.

@mgold
Copy link
Contributor

mgold commented Nov 20, 2015

Looks like it's a matter of waiting for Laszlo to merge, which he should in the next day. I monkey patched my version and can continue testing. Thanks again!

@jackfranklin
Copy link

@deadfoxygrandpa thank you for your quick efforts here, they are much appreciated :)

@deadfoxygrandpa
Copy link
Owner

@laszlopandy just updated this. If you're still having issues, wipe out your elm-stuff directory and run elm package install again. It should work.

I'll publish a minor update to elm-test to increase the dependence on elm-console to 1.0.3 so this won't be able to affect people again.

deadfoxygrandpa added a commit that referenced this issue Nov 24, 2015
@laszlopandy
Copy link
Contributor

Thanks Alex and sorry for the delay everyone!

@deadfoxygrandpa
Copy link
Owner

No problem Laszlo, it was over the weekend and only a couple days.

igrep added a commit to igrep/igrep-cashbook that referenced this issue Nov 24, 2015
@igrep
Copy link
Author

igrep commented Nov 24, 2015

Thank you guys! 😺 I appreciate it!

@mgold
Copy link
Contributor

mgold commented Nov 24, 2015

Bumping the dependency is a good idea, but even so elm-package will pull down the latest elm-console within version bounds right?

@deadfoxygrandpa
Copy link
Owner

Yes, it will, but this way it explicitly disallows a broken combination, and also if someone was affected by this issue and isn't reading this github issue then they can try updating elm-test and it will also pull the new elm-console dependency.

@greyepoxy
Copy link

Hey @deadfoxygrandpa and @laszlopandy I still see this issue when compiling my test code with webpack via the elm-webpack-loader. Seems like the fix in elm-console just checks to see if module and module.exports are defined.

if (typeof module !== 'undefined' && module.exports) {
    fs = require('fs');
...

The elm webpack loader does end up adding module.exports = Elm; to the compiled elm file so this check passes through in the browser.

I have not tried a fix yet but I am hoping that just checking if require is defined should work.

After investigating this issue I was left with this question. Does it make sense for the test runners to be part of this package? It seems like users that want to run their tests in both the browser and in node would need two elm main files anyway why not skip this whole class of problems by removing the need to load node specific code in the browser at all?
To be more specific about what I mean, remove the current test runners, and then add a new runner function that takes some kind of test results displayer function. Or just return the test results and then users will map to their required output.

Either way, Thanks for the great package!

@greyepoxy
Copy link

I did give it a try with an additional check to see if require is defined and it worked,

if (typeof module !== 'undefined' && module.exports && typeof require !== 'undefined') {
    fs = require('fs');
...

@laszlopandy I can open an issue and submit a pull request if you would like.

@laszlopandy
Copy link
Contributor

If you update to 1.1.1 it should fix this issue. It only checks if require is undefined. So if it is defined on the window (by require.js for example) this solution will not work. Please file another issue if that is the case.
http://package.elm-lang.org/packages/laszlopandy/elm-console/1.1.1/

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

7 participants