SSR transformed named imported functions don't work with debuggers "step-into"

[AriPerkkio]

Describe the bug

Originally reported at vitest-dev/vitest#5569. I think I've seen 2-3 similar reports in the past too.

When a file is SSR transformed in Vite, debuggers like Chrome Dev tools and VSCode are unable to "step into" functions that are imported via named imports. In practice this means stopping debugger in line 3 and pressing "step into" button of debuggers in the example below.

1 | import { add } from "./math.ts";
2 |
3 | add(1, 2);

Debugger should stop at line 1 or 2, but instead it stops at 7.

1 | export function add(a: number, b: number): number {
2 |   if (a === 1) {
3 |     return 1;
4 |   }
5 | 
6 |   return a + b;
7 | }
8 |

Source map of the math.ts in reproduction: https://evanw.github.io/source-map-visualization/#NDI2AGZ1bmN0....

Video demonstrating step-into issue with Vite SSR

vite-step-into.webm

I think this has something to do with the way Vite SSR transforms import into import() and saves the results into single variable. In the reproduction setup I've created "Vite simulation", where SSR transformed file is loaded directly into Node.

In this example "step into" works correctly on lines 11 and 15. If Vite was able to destructure the named imports of __vite_ssr_exports__ into separate variables, I think debuggers would work just fine. But this would break ESM live bindings.

 1 | globalThis.__vite_ssr_exports__ = {};
 2 | debugger;
 3 | 
 4 | await import("./transpiled.js");
 5 | const { add, multiply } = globalThis.__vite_ssr_exports__;
 6 | 
 7 | // Incorrect position when "step into"
 8 | globalThis.__vite_ssr_exports__.add(1, 2);
 9 | 
10 | // Correct position when "stepinto"
11 | add(1, 2);
12 | 
13 | // Same here
14 | globalThis.__vite_ssr_exports__.multiply(1, 2);
15 | multiply(1, 2);
16 |

Video showing how lines 11 and 15 work correctly

node-step-into.webm

Reproduction

https://github.com/AriPerkkio/vite-ssr-transform-sourcemaps-issue

Steps to reproduce

See README https://github.com/AriPerkkio/vite-ssr-transform-sourcemaps-issue/blob/main/README.md.

System Info

System:
    OS: macOS 15.0.1
    CPU: (8) arm64 Apple M2
    Memory: 471.58 MB / 16.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 20.17.0 - ~/.nvm/versions/node/v20.17.0/bin/node
    Yarn: 1.22.22 - ~/.nvm/versions/node/v20.17.0/bin/yarn
    npm: 10.8.2 - ~/.nvm/versions/node/v20.17.0/bin/npm
    pnpm: 8.15.9 - ~/.nvm/versions/node/v20.17.0/bin/pnpm
  Browsers:
    Chrome: 129.0.6668.91
    Safari: 18.0.1

Used Package Manager

pnpm

Logs

No response

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[AriPerkkio]

In the source map there is a weird mapping into the position where debugger stops. I tested if removing this with magic-string + @ampproject/remapping helps, but debugger is still working the same.

• Before: https://evanw.github.io/source-map-visualization/#NDI2...
  

• After: https://evanw.github.io/source-map-visualization/#NDI2...
  

[sapphi-red]

I guess we need to tell the debugger that the property access of __vite_ssr_import_0__.add should be ignored when stepping. But I'm not sure if there's a way to do that.

One idea would be to avoid the property access with a helper function and map that helper function to somewhere and set that in x_google_ignoreList.

const access = (obj, key) => obj[key] // the helper function

access(__vite_ssr_import_0__, 'add')(1, 2)

I'm not sure if that would work though.

[sheremet-va]

Can we transform the code into something like this? Instead of letting the getter keep track of the override, we keep track of it ourselves since this is statically analysable.

// source code
export function test() {}
test = function override() {}

// ssr transform
function test() {}
__vite_ssr_exports__.test = test
test = __vite_ssr_exports__.test = function override() {}

There are some edge cases like ([test] = [function name(){}]), we can also support those. The number of reassignment operations is finite 😄

Or just have a helper that syncs the value:

function test() {}
__vite_ssr_define_export__('test', test)
test = function override() {}
__vite_ssr_define_export__('test', test)

This should remove the debugger issue and the getter access overhead that we see in benchmarks:

• "import" is the usual Vite SSR import
• "import-reassign" is storing the Vite SSR import in a local variable so getter it not invoked every time
• "import-getter" is actually accessing a property on the object, not a getter

https://stackblitz.com/edit/vitest-dev-vitest-icireq?file=src%2Fbasic.bench.ts

The main issue with this approach is how do we seal the namespace object then? 🤔

[sapphi-red]

Oh, sorry, I wrote __vite_ssr_exports__ where I should write __vite_ssr_import_0__ in the previous comment. I updated the comment.
I think the problem here is on the importer side rather than the importee side. Even if we change the exports as you proposed, I guess we still need to access the exported values from other files with __vite_ssr_import_0__.add to make live bindings work. In that case, we still have a property access and I guess the debugger stops there.

[hi-ogawa]

I tested Ari's reproduction with 6.0.0-beta.10 and now it steps back and forth once more due to __vite_ssr_identity__. I wonder if this also adds up during bench 🤔

It looks like, in principle, using Object.defineProperty(... value: ...) instead of getter and removing __vite_ssr_identity__ is enough to make debugger step-into properly. I just tested with local build of #18725

The main issue with this approach is how do we seal the namespace object then? 🤔

If we use a plain object without sealing, would that make it faster than normal esm? That can maybe skew benchmark in an opposite way 😄

[hi-ogawa]

Webpack also uses getter between modules during dev and they seem to have a same issue (though I couldn't find a report on their repo). I made a repro here https://github.com/hi-ogawa/reproductions/tree/main/webpack-debugger-step-into
(Maybe it can happen on production too for modules between chunks since module concatenation/hoisting is only within a same chunk.)

[sheremet-va]

It looks like, in principle, using Object.defineProperty(... value: ...) instead of getter and removing __vite_ssr_identity__ is enough to make debugger step-into properly. I just tested with local build of #18725

That is what I was implying in my message. The issue is not with how it's accessed (local variable or property access), but with the getter. And now __vite_ssr_identity__ adds additional overhead. Do we really need to use __vite_ssr_identity__? Can't we just bind the context?

function foo() {
  console.log(this)
  return this
}
Object.defineProperty(__vite_ssr_exports__, "foo", { enumerable: true, configurable: true, value: foo.bind(undefined) });

If we use a plain object without sealing, would that make it faster than normal esm? That can maybe skew benchmark in an opposite way 😄

I don't think we can match the performance of normal ESM, to be honest, it has access to the native APIs that can speed things up with static imports. All we need to do is make this use case consistent:

import { Link } from 'module'
console.log(Link)

import { Dependency } from 'module'
console.log(Dependency.link)

Also, just a note, but it seems like it should generally take ~30,000,000 ops/sec:

┌─────────┬──────────────┬──────────────┬───────────────────┬──────────┬─────────┐
│ (index) │ Task Name    │ ops/sec      │ Average Time (ns) │ Margin   │ Samples │
├─────────┼──────────────┼──────────────┼───────────────────┼──────────┼─────────┤
│ 0       │ 'esm import' │ '30,056,562' │ 33.27060399999566 │ '±0.81%' │ 1000000 │
└─────────┴──────────────┴──────────────┴───────────────────┴──────────┴─────────┘

direct-access is a bit slower, and import-reassign is a bit faster than that 😄

Details

```js import { Bench } from 'tinybench' import { squared } from './basic.mjs'

const bench = new Bench({ name: 'esm import', time: 0, iterations: 1000_000 })

bench.add('esm import', () => {
squared(2)
})
await bench.run()
console.table(bench.table())

</details>

[sheremet-va]

A note about ESM sealing - ES Module Namespace is sealed (Object.seal), not frozen (Object.freeze), meaning we can't add new properties or delete existing ones, but we can override the values (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import#module_namespace_object). But all properties are also read-only 😄

(I also just noticed that the keys are not sorted, and the descriptor is not correct - it should say writable, but still throw an error on writing)

[sheremet-va]

What if we use a different object inside the original module, and the public namespace object just inherits it as a prototype?

const __vite_ssr_exports__ = Object.create(null) // private module
function foo() {}
Object.defineProperty(__vite_ssr_exports__, 'foo', { configurable: false, writable: true, value: foo })
foo = function newFoo() {}
Object.defineProperty(__vite_ssr_exports__, 'foo', { configurable: false, writable: true, value: foo })

const __vite_ssr_import_0__ = await __vite_ssr_import__('./foo.js')
// this return `__vite_ssr_public_exports__` instead of `__vite_ssr_exports__`
const __vite_ssr_public_exports__ = Object.freeze(Object.create(__vite_ssr_exports__))

That way we can modify the values on __vite_ssr_exports__ but users still can't change it on __vite_ssr_public_exports__ because the object is frozen

Playground: https://stackblitz.com/edit/node-lrku7j?file=index.js

Issues with this approach:

• Object.getPrototypeOf(module) is an object __vite_ssr_exports__ instead of null
• Object.keys() doesn't return anything
• __vite_ssr_public_exports__.reassignedValue = 1 returns a different error than if it was read-only
• Couldn't benchmark this reliably - sometimes the prototype approach is much faster than the rest, but sometimes it's two times slower 😄

All of these are fixable with a proxy that doesn't have a get handler to avoid stepping into:

const originalModule = Object.create(__vite_ssr_exports__)
Object.defineProperty(originalModule, Symbol.for('nodejs.util.inspect.custom'), {
  writable: false,
  enumerable: false,
  value: function () {
    const utils = require('node:util');
    return utils.format(__vite_ssr_exports__);
  },
});
const mod = new Proxy(originalModule, {
  set(_, p) {
    const key = String(p);
    if (!Reflect.has(__vite_ssr_exports__, p)) {
      throw new TypeError(
        `Cannot add property ${key}, object is not extensible`
      );
    }
    throw new TypeError(
      `Cannot assign to read only property '${key}' of object 'Module'`
    );
  },
  defineProperty(_, p) {
    const key = String(p);
    if (!Reflect.has(__vite_ssr_exports__, k)) {
      throw new TypeError(
        `Cannot define property ${key}, object is not extensible`
      );
    }
    throw new TypeError(`Cannot redefine property: ${key}`);
  },
  setPrototypeOf() {
    throw new TypeError('Module is not extensible');
  },
  getPrototypeOf() {
    return null;
  },
  getOwnPropertyDescriptor(_, p) {
    return Reflect.getOwnPropertyDescriptor(__vite_ssr_exports__, p);
  },
  has(_, p) {
    return Reflect.has(__vite_ssr_exports__, p);
  },
  ownKeys() {
    return [...Refect.ownKeys(__vite_ssr_exports__), Symbol.for('nodejs.util.inspect.custom')];
  },
  isExtensible() {
    return false;
  },
  preventExtensions() {
    return true;
  },
});

Another problem arises - Object.isSealed(mod) returns false 😞

Playground with a proxy: https://stackblitz.com/edit/node-7j9afi?file=index.js

[sheremet-va]

What we can definitely do right now without any breaking change is to have Object.defineProperty(... value: ...) for any export that we know is not reassigned and doesn't need live-binding. This would at least improve the situation for most users since live-binding is not a very popular feature.

[sheremet-va]

I think I found solution that works identical to ESM in Node.js (not in stackblitz for some reason - looks like they are using a vite-node like wrapper to import 😄 )

https://stackblitz.com/edit/node-7j9afi?file=test.js

Running this code in Node.js shows the same values for the native ESM module and the proxy+wrapper. One caveat - to make this work, we need to know all exports before we execute the code (we can just return them alongside dependencies)

// exports are statically known, fill the object before executing the code
const __vite_ssr_exports__ = {
  a: undefined,
  __proto__: null,
};
Object.defineProperty(__vite_ssr_exports__, Symbol.toStringTag, {
  enumerable: false,
  writable: false,
  configurable: false,
  value: 'Module',
});

const __vite_ssr_namespace__ = new Proxy(__vite_ssr_exports__, {
  set(target, p) {
    const key = String(p);
    if (!Reflect.has(target, p)) {
      throw new TypeError(
        `Cannot add property ${key}, object is not extensible`
      );
    }
    throw new TypeError(
      `Cannot assign to read only property '${key}' of object 'Module'`
    );
  },
});

// this also seals the __vite_ssr_exports__, but sealed objects CAN override values
Object.seal(__vite_ssr_namespace__);
// this is LEGAL and the only LEGAL way to reassign values
// we can call this INSIDE the module
__vite_ssr_exports__.a = 123;
// this is NOT LEGAL (because of the proxy),
// this is the module exposed to the user
__vite_ssr_namespace__.a = 123;

// this returns descriptors ACCORDING to the MDN (writable: true)
console.log(Object.getOwnPropertyDescriptors(__vite_ssr_namespace__))

The new algorithm would be something like this:

• Parse and transform the file as usual, but
  • Keep a record of all exports
  • Use = assignment to assign values instead of Object.defineProperty because the module will be sealed before the values are assigned at runtime
  • Add reassignments of __vite_ssr_exports__.${method} when the export is reassigned
  • Instead of using __vite_ssr_identity__ use (0, method)()
• Create the __vite_ssr_exports__ objects with all exported properties assigned to undefined (we kept the record of all exports) and sort them
• Create a public proxy __vite_ssr_namespace__ that throws an error if someone attempts to assign a value
• __vite_ssr_import__ now returns __vite_ssr_namespace__ instead of __vite_ssr_exports__ for public usage

Benchmark-wise it seems to be the slowest one 😄 (Even slower than getters, even though we don't define a get method on a proxy). Property access seems to be on par with reassignment (I recommend running benchmarks locally and not on stackblitz because it messes with imports):

In real-world scenarios using the module runner, it makes it around ~4 mil operations slower:

Details

const server = await createServer({
  root: process.cwd(),
})

const AsyncFunction = async function () {}.constructor as typeof Function

class MyEvaluator {
  startOffset: number
  runExternalModule: (url: string) => Promise<any>

  constructor() {
    const evaluator = new ESModulesEvaluator()
    this.runExternalModule = evaluator.runExternalModule.bind(evaluator)
    this.startOffset = evaluator.startOffset
  }

  async runInlinedModule(context: ModuleRunnerContext, code: string, module: EvaluatedModuleNode): Promise<any> {
    context[ssrModuleExportsKey].squared = undefined
    const __vite_ssr_namespace__ = new Proxy(context[ssrModuleExportsKey], {
      set(target, p) {
        const key = String(p);
        if (!Reflect.has(target, p)) {
          throw new TypeError(
            `Cannot add property ${key}, object is not extensible`
          );
        }
        throw new TypeError(
          `Cannot assign to read only property '${key}' of object 'Module'`
        );
      },
    });
    module.exports = __vite_ssr_namespace__
    Object.seal(__vite_ssr_namespace__)
    const transform = code.replace('Object.defineProperty(__vite_ssr_exports__, "squared", { enumerable: true, configurable: true, get(){ return squared }});', '__vite_ssr_exports__.squared = squared;')
    console.log(transform)
    // use AsyncFunction instead of vm module to support broader array of environments out of the box
    const initModule = new AsyncFunction(
      ssrModuleExportsKey,
      ssrImportMetaKey,
      ssrImportKey,
      ssrDynamicImportKey,
      ssrExportAllKey,
      // source map should already be inlined by Vite
      '"use strict";' + transform,
    )

    await initModule(
      context[ssrModuleExportsKey],
      context[ssrImportMetaKey],
      context[ssrImportKey],
      context[ssrDynamicImportKey],
      context[ssrExportAllKey],
    )
  }
}

class OverrideModuleRunner extends ModuleRunner {
  override async directRequest(
    url: string,
    mod: EvaluatedModuleNode,
    _callstack: string[],
  ) {
    await super.directRequest(url, mod, _callstack)
    return mod.exports
  }
}

const createServerModuleRunnerTransport = (options: {
  channel: any
}): ModuleRunnerTransport => {
  const hmrClient: HotChannelClient = {
    send: (payload: HotPayload) => {
      if (payload.type !== 'custom') {
        throw new Error(
          'Cannot send non-custom events from the client to the server.',
        )
      }
      options.channel.send!(payload)
    },
  }

  let handler: ((data: HotPayload) => void) | undefined

  return {
    connect({ onMessage }) {
      options.channel.api!.outsideEmitter.on('send', onMessage)
      onMessage({ type: 'connected' })
      handler = onMessage
    },
    disconnect() {
      if (handler) {
        options.channel.api!.outsideEmitter.off('send', handler)
      }
    },
    send(payload) {
      if (payload.type !== 'custom') {
        throw new Error(
          'Cannot send non-custom events from the server to the client.',
        )
      }
      options.channel.api!.innerEmitter.emit(
        payload.event,
        payload.data,
        hmrClient,
      )
    },
  }
}

const runner = new OverrideModuleRunner(
  {
    root: server.config.root,
    transport: createServerModuleRunnerTransport({
      channel: server.environments.ssr.hot
    })
  },
  new MyEvaluator()
)

const defaultRunner = createServerModuleRunner(server.environments.ssr)

const module = await runner.import('/basic.mjs')

bench(
  'proxy-access',
  () => {
    module.squared(2)
  },
  config
);

const module2 = await defaultRunner.import('/basic.mjs')

bench(
  'direct-access',
  () => {
    module2.squared(2);
  },
  config
);

Just a note: internally, module namespace exports are defined as getters/setters: https://github.com/nodejs/node/blob/main/deps/v8/src/builtins/accessors.cc#L265

[sapphi-red]

That is what I was implying in my message. The issue is not with how it's accessed (local variable or property access), but with the getter.

Ah, I wasn't understanding that.

Do we really need to use __vite_ssr_identity__? Can't we just bind the context?

I didn't come up with that. I guess it needs to be like:

Object.defineProperty(__vite_ssr_exports__, "foo", {
  enumerable: true,
  configurable: true,
  // use Function.prototype.bind to make functions with null prototype work
  // check if it's a function in runtime (as we cannot statically analyze it, see the code below)
  value: typeof foo === 'function' ? Function.prototype.bind.call(foo, undefined) : foo;
});

// foo.mjs
import { a, update } from './bar.mjs';

console.log(a); // function
update();
console.log(a); // 123

// bar.mjs
export function a() {}

export function update() {
  a = 123;
}

• Instead of using __vite_ssr_identity__ use (0, method)()

I avoided this because I didn't know when I need to inject ; before (0, method)() to prevent foo(0, method)() from happening.

Benchmark-wise it seems to be the slowest one 😄

It'd be nice to compare the real world case. For example, running vitest in some repo and check the time it takes. If it has big difference, maybe we can only use proxy if debugger is enabled 🤔 That could make the behavior confusing though...