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

Browserified jsdom cannot execute <script> tags #1023

Closed
wants to merge 2 commits into from

Conversation

Sebmaster
Copy link
Member

Our contextify-shim.js doesn't even have a run function. I tried to add one, just

  o.run = function (code, filename) {
    o.eval(code + "\n//# sourceURL=" + filename);
  };

but this of course doesn't work since it's running the code in the global scope (e.g. in a worker), where variables like window and document don't work right.

@Sebmaster
Copy link
Member

I did a thing, it works, pls comment.

See https://travis-ci.org/tmpvar/jsdom/builds/50547517 for the proper build with browser test.

@domenic
Copy link
Member Author

domenic commented Feb 12, 2015

Wow, looks pretty awesome :D. Maybe add a test for async access within some nested scopes?


var globals = findGlobals(ast);
for (var i = 0; i < globals.length; ++i) {
if (globals[i].name === "window") continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curlies and newline. JSHint should be catching this? Can we JSHint this file now?

@Sebmaster Sebmaster force-pushed the fix/execution-context branch from f0e129c to 5ef69ce Compare February 12, 2015 17:52
@Sebmaster
Copy link
Member

Fixed the nitpicks, but need to get some sleep before I add a test. Will do later tonight though.

@Sebmaster Sebmaster force-pushed the fix/execution-context branch 2 times, most recently from 1674775 to f75ee37 Compare February 12, 2015 21:00
@Sebmaster
Copy link
Member

Added the async test.

@Sebmaster Sebmaster modified the milestone: 3.x Feb 20, 2015
This dynamically rewrites the provided source code to always refer to window if it's a global variable.

Caveats:
* toString() of functions will return different results: a "window." prefix on global vars and whitespace modifications
* Refering to undefined variables will not throw an error due to the window prefix for now. This might be fixable by throwing the error ourselves.
We can successfully run more parts of our node test suite on the browser too now!
Also updated a few of the comments of the suites I've tried.
@Sebmaster Sebmaster force-pushed the fix/execution-context branch from f75ee37 to 4546b8d Compare February 20, 2015 18:11
@domenic
Copy link
Member Author

domenic commented Feb 20, 2015

Fixed, at least somewhat, with 0ec0d0e. Will try to track down more cases as they pop up.

@domenic domenic closed this Feb 20, 2015
@Sebmaster Sebmaster deleted the fix/execution-context branch February 20, 2015 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants