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

Add detection for whether we are on an active Stopify stack #514

Merged
merged 2 commits into from
Jul 31, 2024

Conversation

jpolitz
Copy link
Collaborator

@jpolitz jpolitz commented Jul 30, 2024

(Sorry about accidentally creating a branch on the main Stopify repo; just meant to make this PR)

Adds a stackActive field to abstractRuntime that tracks whether the JavaScript stack is currently using Stopified frames, and exposes it through isRunning() on abstractRuntime.

The use case for this, beyond it being a likely-useful utility, is writing polyfills for HOFs that can switch on and off depending on if they can expect to capture or not.

We've done some of this work for Pyret
(https://github.com/brownplt/pyret-lang/blob/8e0ce78fb0ca1c10bcc06dfcaeb534d0ae2c02e4/src/runtime/hof-array-polyfill.ts#L347) because the Pyret runtime co-exists on a page with regular old React code.

Because everything in React and in the Stopified runtime is getting asynchronously chopped up and scheduled all over the place, and because we want JS arrays to be arrays whether in the stopify runtime or the page runtime, it's useful to have polyfills that automatically do the right thing. This avoids these problems:

  • If using the default array polyfill strategy from Stopify, non-stopified code can get access to arrays with stopified map/filter/fold. Say that's called in some didComponentUpdate or other scheduled React event – now it's not on a Stopify stack and lots of e.g. uncaught Captures result. It is really, really hard to remember that every array may or may not be from the Stopify runtime and have a different prototype.
  • When calling into Stopified code, it's natural to want to pass in “regular” arrays. It's also hard to remember and design APIs around introducing wrapping on these as they enter stopified code.
  • The flag lets us handle cases like:
    • A didComponentUpdate starts a Pyret evaluation for e.g. rendering a Pyret value to a React element, which calls back into some Stopified Pyret code. This goes in a suitable wrapper.
    • That code suspends for whatever reason, yielding control back to didComponentUpdate, which uses e.g. map/filter/fold. The map/filter/fold in didComponentUpdate will correctly use the plain, un-instrumented map/filter/fold.
    • The Stopified code resumes in the next turn and does map/filter/fold in the dynamic extent of the Pyret code. This correctly uses the stopified versions of the HOFs.

Adds a stackActive field to abstractRuntime that tracks whether the JavaScript
stack is currently using Stopified frames, and exposes it through isRunning()
on abstractRuntime.

The use case for this, beyond it being a likely-useful utility, is writing
polyfills for HOFs that can switch on and off depending on if they can expect
to capture or not.

We've done some of this work for Pyret
(https://github.com/brownplt/pyret-lang/blob/8e0ce78fb0ca1c10bcc06dfcaeb534d0ae2c02e4/src/runtime/hof-array-polyfill.ts#L347)
because the Pyret runtime co-exists on a page with regular old React code.

Because everything in React and in the Stopified runtime is getting
asynchronously chopped up and scheduled all over the place, and because we want
JS arrays to be arrays whether in the stopify runtime or the page runtime, it's
useful to have polyfills that automatically do the right thing. This avoids
these problems:

- If using the default array polyfill strategy from Stopify, non-stopified code
  can get access to arrays with stopified map/filter/fold. Say that's called in
  some `didComponentUpdate` or other scheduled React event – now it's not on a
  Stopify stack and lots of e.g. uncaught Captures result. It is really, really
  hard to remember that every array may or may not be from the Stopify runtime
  and have a different prototype.
- When calling into Stopified code, it's natural to want to pass in “regular”
  arrays. It's also hard to remember and design APIs around introducing
  wrapping on these as they *enter* stopified code.
- The flag lets us handle cases like:
  - A didComponentUpdate starts a Pyret evaluation for e.g. rendering a Pyret
    value to a React element, which calls back into some Stopified Pyret code.
    This goes in a suitable wrapper.
  - That code suspends for whatever reason, yielding control back to
    didComponentUpdate, which uses e.g. map/filter/fold. The map/filter/fold in
    didComponentUpdate will correctly use the plain, un-instrumented
    map/filter/fold.
  - The Stopified code resumes in the next turn and does map/filter/fold in the
    dynamic extent of the Pyret code. This correctly uses the stopified
    versions of the HOFs.
@jpolitz
Copy link
Collaborator Author

jpolitz commented Jul 30, 2024

CC @blerner

@jpolitz
Copy link
Collaborator Author

jpolitz commented Jul 30, 2024

And just to be clear, we want to replace the code for isStopifyRunning in hof-array-polyfill with just $STOPIFY.isRunning(). The eventMode field isn't actual appropriate for this purpose all the time, and it would be nice to have this in an API that Stopify usefully tracks with its internal state.

@arjunguha
Copy link
Member

er, I just merged the other one. should I update and rebase?

I can try to get the github actions test cases to work again. Not sure why we were running self-hosted.

@arjunguha
Copy link
Member

I spent some time in a rabbit hole trying to update the integration tests from selenium-webdriver to puppeteer. To do so, I need to also update the TypeScript used, which at this point is a bit ancient. I am willing to try, eventually.

Maybe I should merge your stuff without worrying about integration tests passing. OK with that?

@jpolitz
Copy link
Collaborator Author

jpolitz commented Jul 31, 2024

Yeah I'm OK with that. I should have this all reverted now so the two PRs are disjoint.

@jpolitz
Copy link
Collaborator Author

jpolitz commented Jul 31, 2024

I got nearly everything in npm test to run by updating the webdriver version to newer Chrome. Just some tests that expect Firefox are not running with a very descriptive error about that.

@arjunguha arjunguha merged commit d35d329 into nuprl:master Jul 31, 2024
1 check failed
@jpolitz jpolitz deleted the activeStack branch July 31, 2024 23:27
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