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

Calling createTSClient stops console.log functioning #101

Open
mcintyre94 opened this issue Oct 20, 2021 · 2 comments
Open

Calling createTSClient stops console.log functioning #101

mcintyre94 opened this issue Oct 20, 2021 · 2 comments

Comments

@mcintyre94
Copy link
Contributor

mcintyre94 commented Oct 20, 2021

Describe the bug

When a page calls createTSClient from @run-wasm/ts its console.log function is silently broken. This is because that functions calls the TSClient constructor:

  public constructor(protected ts: any) {
    // Overriding the console.log method so that we can store the logs
    console.oldLog = console.log
    console.log = (value) => {
      // For some reason, the first 'incoming' log is always an instance of TSClient. Ignoring it
      if (!(value instanceof TSClient)) {
        this.logs.push(value)
      }
    }
  }

This is not sandboxed in any way by the browser, so calling it overrides console.log with a function that no longer logs messages

To Reproduce

I've created a replit showing this: https://ConsoleLogOverride.mcintyre94.repl.co

Replit seems to have an issue loading ATM so you may need to use the edit link: https://replit.com/@mcintyre94/ConsoleLogOverride#pages/index.js (can run the app from here)

  • Open the browser console
  • Click the top button "Log something!" a few times
  • It logs a unique message each click
  • Click the second button "Load TS Client", this calls createTSClient
  • The log message "About to call createTSClient!" appears
  • The log message "Now called createTSClient!" does not appear
  • Click the top button "Log something!" again
  • It will no longer log messages because console.log has been overwritten

Expected behavior

Calling createTSClient should not affect the caller's ability to log.

I'm not 100% sure but maybe we should have the overwritten console.log also call console.oldLog? This would mean that logs in the editor are printed to the browser console as well as captured to be displayed as output - probably not a bad thing? That'd be a one line fix and we wouldn't need to try to figure out a way to isolate the console change, which I suspect we can't do in the browser context anyway.

Using console.oldLog(...) in the caller works correctly as a workaround for now.

Screenshots

Screenshot 2021-10-20 at 23 01 37

Desktop (please complete the following information):

  • OS: macOS
  • Browser: Vivaldi
  • Version: 4.3.2439.44

Additional context

The page in my replit has the following code:

import Head from 'next/head'
import Image from 'next/image'
import styles from '../styles/Home.module.css'
import { createTSClient } from '@run-wasm/ts'
import { useEffect } from 'react'

export default function Home() {
  const logMessage = () => {
    console.log(`You have clicked a button ${Math.random()}`)
  }

  const loadTSClient = () => {
    const ts = {}
    console.log('About to call createTSClient!')
    createTSClient(ts)
    console.log('Now called createTSClient!')
  }

  return (
    <>
    <button onMouseDown={logMessage}>Log something!</button>
    <br />
    <button onMouseDown={loadTSClient}>Load TS Client</button>
    </>
  )
}
@mcintyre94
Copy link
Contributor Author

Thinking a bit more about this I think we have a few options here.

  • We could make the overwritten console.log also write to the console, my initial suggestion. The constructor becomes something like:
  public constructor(protected ts: any) {
    // Overriding the console.log method so that we can store the logs
    console.oldLog = console.log
    console.log = (value) => {
      // For some reason, the first 'incoming' log is always an instance of TSClient. Ignoring it
      if (!(value instanceof TSClient)) {
        console.oldLog(value)
        this.logs.push(value)
      }
    }
  }

This should work, in that if the page embeds run-wasm and calls console.log it behaves as expected. However we'll still be pushing console.log calls from the page into this.logs. While we clear logs (though caveat: also see #102 ) before running the code, if the page (or anything else on it) is logging while the code is running then everything is going to end up in the code output. That could be really confusing!

  • We could instead define a new function, eg. console.recordLog. Then when we run code we could s/console.log/console.recordLog/g before running it. But that's limited because of course you can have code like x=console; x.log(...) and then those messages aren't going to end up in this.logs. This is the only solution I can think of that won't interleave unrelated log calls into this.logs though.

  • We could override console.log only in the function where we run code, and change it back to console.oldLog afterward. That would mean that when we're not running code, logs work fine from the page. But if the page logs while code is running then it's still going to end up in this.logs. We could of course combine this with the first option and have those captured log messages also printed to the console as usual.

My concern with anything except the second option is that some pages like to do things like log every redux action, and even if the page isn't logging you can't trust third party libraries not to. Mixing page logs and code logs in output has the potential to be really confusing. Maybe it won't be an issue in practice though? And the second option feels limiting, especially in the context of teaching.

Anyone else got any thoughts or solutions that I haven't thought of?

@kennethcassel
Copy link
Contributor

I started looking around at how other people are handling this for TS playgrounds and found this same category of issues happening with the folks building the Microsoft ts playground.

Taking a look at how they approach the problem might be useful!
microsoft/TypeScript-Website#1390

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

2 participants