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

Fix examples involving script for the JS browser in chapter 9+ #1346

Merged
merged 2 commits into from
Sep 20, 2024

Conversation

chrishtr
Copy link
Collaborator

@chrishtr chrishtr commented Sep 13, 2024

There were several bugs:

  1. The lab*.hints files for certain chapters unnecessarily hinted that open should map to filesystem.open. While that is correct, it caused code in compiler.py to detect calls to open and insert files on disk into the top of the JS file.
  2. lab13.hints had incorrect hints for dispatching JS. Since the code was unchanged from lab9, I just deleted it.
  3. lab*-browser.html for chapters 12+ reset needs_animation_frame after calling render rather than before. This is incorrect if rAF callbacks set it during render's callouts to JS.
  4. localhost was not whitelisted for allowing direct fetching from rt.js (only affects debugging flows).

For good measure, I modified rt.js to special-case URLs with "example" in them so that they didn't cause localhost URLs to end up going to server*.js. (Alternatively one could run the localhost server at a port other than 8000.)

Now examples/lab13-opacity-raf.html works correctly in the JS cross-compiled browser.

@chrishtr
Copy link
Collaborator Author

?

Comment on lines 18 to +19
console.log('Script crashed');
console.log(e);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah this is fine but we can probably use like console.trace or console.error, right? Not really worth optimizing I guess.

@@ -117,7 +117,8 @@ class socket {
let [line1] = this.input.split("\r\n", 1);
let [method, path, protocol] = line1.split(" ");
this.url = this.scheme + "://" + this.host + path;
if (this.host == "localhost" && rt_constants.URLS["local://" + this.port]) {
if (this.host == "localhost" && rt_constants.URLS["local://" + this.port] &&
path.indexOf("example") == -1) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Smart

@pavpanchekha pavpanchekha merged commit fa263b0 into main Sep 20, 2024
1 check passed
@pavpanchekha pavpanchekha deleted the fixexamples branch September 20, 2024 20:06
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

Successfully merging this pull request may close these issues.

2 participants