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: Add globalPreload to ts-node/esm for node 20 #2009

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Commits on Sep 7, 2023

  1. remove swc experimental options

    This removes support for keeping import assertions, which were broken in
    swc at some point, and unconditionally transpiled into import
    attributes. (Ie, `import/with` instead of `import/assert`.)
    
    No version of node supports import attributes with this syntax yet, so
    anyone using swc to import json in ESM is out of luck no matter what.
    
    And swc 1.3.83 broke the option that ts-node was using. The position of
    the swc project is that experimental features are not supported, and may
    change in patch versions without warning, making them unsafe to rely on
    (as evidenced here, and the reason why this behavior changed
    unexpectedly in the first place).
    
    Better to just not use experimental swc features, and let it remove
    import assertions rather than transpile them into something that node
    can't run.
    
    Fix: TypeStrong#2056
    isaacs committed Sep 7, 2023
    Configuration menu
    Copy the full SHA
    670d0ea View commit details
    Browse the repository at this point in the history
  2. fix: Add globalPreload to ts-node/esm for node 20

    As of node v20, loader hooks are executed in a separate isolated thread
    environment.  As a result, they are unable to register the
    `require.extensions` hooks in a way that would (in other node versions)
    make both CJS and ESM work as expected.
    
    By adding a `globalPreload` method, which *does* execute in the main
    script environment (but with very limited capabilities), these hooks can
    be attached properly, and `--loader=ts-node/esm` will once again make
    both cjs and esm typescript programs work properly.
    isaacs committed Sep 7, 2023
    Configuration menu
    Copy the full SHA
    afbd4b6 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    71e319c View commit details
    Browse the repository at this point in the history
  4. fix: ts-node --esm on Node v20

    When running `ts-node --esm`, a child process is spawned with the
    `child-loader.mjs` loader, `dist/child/child-entrypoint.js` main,
    and `argv[2]` set to the base64 encoded compressed configuration
    payload.
    
    `child-loader.mjs` imports and re-exports the functions defined
    in `src/child/child-loader.ts`. These are initially set to empty
    loader hooks which call the next hook in line until they are
    defined by calling `lateBindHooks()`.
    
    `child-entrypoint.ts` reads the config payload from argv, and
    bootstraps the registration process, which then calls
    `lateBindHooks()`.
    
    Presumably, the reason for this hand-off is because `--loader`
    hooks do not have access to `process.argv`.  Unfortunately, in
    node 20, they don't have access to anything else, either, so
    calling `lateBindHooks` is effectively a no-op; the
    `child-loader.ts` where the hooks end up getting bound is not the
    same one that is being used as the actual loader.
    
    To solve this, the following changes are added:
    
    1. An `isLoaderThread` flag is added to the BootstrapState. If
       this flag is set, then no further processing is performed
       beyond binding the loader hooks.
    2. `callInChild` adds the config payload to _both_ the argv and
       the loader URL as a query param.
    3. In the `child-loader.mjs` loader, only on node v20 and higher,
       the config payload is read from `import.meta.url`, and
       `bootstrap` is called, setting the `isLoaderThread` flag.
    
    I'm not super enthusiastic about this implementation. It
    definitely feels like there's a refactoring opportunity to clean
    it up, as it adds some copypasta between child-entrypoint.ts and
    child-loader.mjs. A further improvement would be to remove the
    late-binding handoff complexity entirely, and _always_ pass the
    config payload on the loader URL rather than on process.argv.
    isaacs committed Sep 7, 2023
    Configuration menu
    Copy the full SHA
    8b4be1d View commit details
    Browse the repository at this point in the history
  5. WIP fix tests for off-thread loader

    cspotcode authored and isaacs committed Sep 7, 2023
    Configuration menu
    Copy the full SHA
    a7aa440 View commit details
    Browse the repository at this point in the history
  6. Pushing my local changes

    cspotcode authored and isaacs committed Sep 7, 2023
    Configuration menu
    Copy the full SHA
    9346c54 View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    db1fdf9 View commit details
    Browse the repository at this point in the history
  8. loader: TSError that survives thread comms channel

    When an error is thrown in the loader thread, it must be passed through
    the comms channel to be printed in the main thread.
    
    Node has some heuristics to try to reconstitute errors properly, but
    they don't function very well if the error has a custom inspect method,
    or properties that are not compatible with JSON.stringify, so the
    TSErrors raised by the source transforms don't get printed in any sort
    of useful way.
    
    This catches those errors, and creates a new error that can go through
    the comms channel intact.
    
    Another possible approach would be to update the shape of the errors
    raised by source transforms, but that would be a much more extensive
    change with further reaching consequences.
    isaacs committed Sep 7, 2023
    Configuration menu
    Copy the full SHA
    d68a150 View commit details
    Browse the repository at this point in the history
  9. loader: Set default 'pretty' option properly

    Set the default `options.pretty` value based on stderr rather than
    stdout, as this is where errors are printed.
    
    The loader thread does not get a process.stderr.isTTY set, because its
    "stderr" is actually a pipe. If `options.pretty` is not set explicitly,
    the GlobalPreload's `context.port` is used to send a message from the
    main thread indicating the state of stderr.isTTY.
    
    Adds `Service.setPrettyErrors` method to enable setting this value when
    needed.
    isaacs committed Sep 7, 2023
    Configuration menu
    Copy the full SHA
    04d9419 View commit details
    Browse the repository at this point in the history
  10. use built-in source map support in node v20

    The @cspotcode/source-map-support module does not function properly on
    Node 20, resulting in incorrect stack traces. Fortunately, the built-in
    source map support in Node is now quite reliable. This does have the
    following (somewhat subtle) changes to error output:
    
    - When a call site is in a method defined within a constructor function,
      it picks up the function name *as well as* the type name and method
      name. So, in tests where a method is called and throws within the a
      function constructor, we see `Foo.Foo.bar` instead of `Foo.bar`.
    - Call sites displays show filenames instead of file URLs.
    - The call site display puts the `^` character under the `throw` rather
      than the construction point of the error object. This is closer to how
      normal un-transpiled JavaScript behaves, and thus somewhat
      preferrable, but isn't possible when all we have to go on is the Error
      stack property, so it is a change.
    
    I haven't been able to figure out why exactly, but the call sites appear
    to be somewhat different in the repl/eval contexts as a result of this
    change. It almost seems like the @cspotcode/source-map-support was
    applying source maps to the vm-evaluated scripts, but I don't see how
    that could be, and in fact, there's a comment in the code stating that
    that *isn't* the case. But the line number showing up in an Error.stack
    property is `1` prior to this change (matching the location in the TS
    source) and is `2` afterwards (matching the location in the compiled
    JS).
    
    An argument could be made that specific line numbers are a bit
    meaningless in a REPL anyway, and the best approach is to just make
    those tests accept either result. One possible approach to provide
    built-in source map support for the repl would be to refactor the
    `appendCompileAndEvalInput` to diff and append the *input* TS, and
    compile within the `runInContext` method. If the transpiled code was
    prepended with `process.setSourceMapsEnabled(true);`, then Error stacks
    and call sites would be properly source mapped by Node internally.
    isaacs committed Sep 7, 2023
    Configuration menu
    Copy the full SHA
    fba2006 View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    5c83838 View commit details
    Browse the repository at this point in the history
  12. add support for '--import ts-node/import'

    This also adds a type for the loader hooks API v3, as globalPreload is
    scheduled for removal in node v21, at which point '--loader ts-node/esm'
    will no longer work, and '--import ts-node/import' will be the way
    forward.
    isaacs committed Sep 7, 2023
    Configuration menu
    Copy the full SHA
    49c74a0 View commit details
    Browse the repository at this point in the history
  13. Configuration menu
    Copy the full SHA
    ed07dd9 View commit details
    Browse the repository at this point in the history
  14. Configuration menu
    Copy the full SHA
    b614b1b View commit details
    Browse the repository at this point in the history