SSR transformed named imported functions don't work with debuggers "step-into" #18325
Comments
|
In the source map there is a weird mapping into the position where debugger stops. I tested if removing this with |
|
I guess we need to tell the debugger that the property access of One idea would be to avoid the property access with a helper function and map that helper function to somewhere and set that in 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. |
|
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 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: 
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? 🤔 |
|
Oh, sorry, I wrote |
|
I tested Ari's reproduction with 6.0.0-beta.10 and now it steps back and forth once more due to It looks like, in principle, using
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 😄 |
|
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 |
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 function foo() {
console.log(this)
return this
}
Object.defineProperty(__vite_ssr_exports__, "foo", { enumerable: true, configurable: true, value: foo.bind(undefined) });
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:
```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', () => { |
|
A note about ESM sealing - ES Module Namespace is sealed ( (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) |
|
What if we use a different object inside the original module, and the public 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 Playground: https://stackblitz.com/edit/node-lrku7j?file=index.js Issues with this approach:
All of these are fixable with a proxy that doesn't have a 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 - Playground with a proxy: https://stackblitz.com/edit/node-7j9afi?file=index.js |
|
What we can definitely do right now without any breaking change is to have |
|
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:
Benchmark-wise it seems to be the slowest one 😄 (Even slower than getters, even though we don't define a
In real-world scenarios using the module runner, it makes it around ~4 mil operations slower:
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 |
Ah, I wasn't understanding that.
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;
}
I avoided this because I didn't know when I need to inject
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... |
We can't just return them because users can do |
|
I started to tinker with your idea. One thing I'm stuck is how to transform renamed re-export? let p = 0;
export { p as q };
p = 1;For the algorithm #18325 (comment), if the replacement is simply based on identifier, this becomes a following and loses local assignment of let p = 0;
__vite_ssr_exports__["q"] = p
__vite_ssr_exports__["q"] = 1; |
|
Oh wait, I guess it's same for simple export too, so we don't replace identifier, but repeat same assignment like this? // input
export let x = 0;
x = 1;// output
let x = 0;
__vite_ssr_exports__["x"] = x;
x = 1; // keep this but detect assignment to x
__vite_ssr_exports__["x"] = x; // then repeatMaybe then this can generalize as: // input
let p = 0;
export { p as q };
p = 1;// output
let p = 0;
__vite_ssr_exports__["q"] = p
p = 1
__vite_ssr_exports__["q"] = p;Now re-reading the algorithm #18325 (comment), I can see that's what's explained 😄 |
|
Another question, is there a way to handle assignment in expression like this? // input
export let x = 0;
someFn(x = 1);// output
let x = 0;
__vite_ssr_exports__["x"] = x;
someFn(x = 1);
__vite_ssr_exports__["x"] = x; // this is technically too lateAlso more complicated pattern is possible here too. export let x = 0;
someFn([x] = [1]); |
Can't you do this? someFn((x = 1, __vite_ssr_exports__["x"] = x)); |
I was thinking export let x = 1;
someFn(x = 1);
someFn([x] = [1, 2]);
someFn({x} = { x: 1, y: 2 });
// output?
someFn((x = 1, __vite_ssr_exports__["x"] = x))
someFn(([x] = [1, 2], __vite_ssr_exports__["x"] = x, [1, 2]))
someFn(({x} = { x: 1, y: 2 }, __vite_ssr_exports__["x"] = x, { x: 1, y: 2 })) |
Maybe like this? - someFn(([x] = [1, 2], __vite_ssr_exports__["x"] = x, [1, 2]))
+ someFn((__vite_ssr_locals__['a0'] = [1, 2], [x] = __a0, __vite_ssr_exports__["x"] = x, __vite_ssr_locals__['a0'])) |
|
// input
let x = 0;
let p = 0;
export { x, p as q };
someFn(x = 1)
someFn([x, p] = [1, 2])
someFn({x, y: p} = { x: 1, y: 2 })
// output
const __vite_ssr_wrap_assign_expr__ = (value) => {
__vite_ssr_exports__["x"] = x;
__vite_ssr_exports__["q"] = p;
return value;
}
someFn(__vite_ssr_wrap_assign_expr__(x = 1));
someFn(__vite_ssr_wrap_assign_expr__([x, p] = [1, 2]));
someFn(__vite_ssr_wrap_assign_expr__({x, y: p} = { x: 1, y: 2 })); |
Does it actually introduce a jump? 🤔 Although, I think I see... Right now, the jump is happening when we access the export here it will happen when we go into a function, I guess, which is very unfortunate I was also thinking about a similar assignment, I guess it will be generated uniquely for every assertion. Or do you just want to list every export in this function and always reassign it? |
I didn't see a problem with reassign everything. Another variant would be chaining them by variables? // output
const __vite_ssr_assign_x_ = (value) => {
__vite_ssr_exports__["x"] = x;
return value;
}
const __vite_ssr_assign_p_ = (value) => {
__vite_ssr_exports__["q"] = p;
return value;
}
someFn(__vite_ssr_wrap_assign_expr_x_(x = 1));
someFn(__vite_ssr_wrap_assign_expr_x__(__vite_ssr_wrap_assign_expr_p_([x, p] = [1, 2])));Also I remember there can be one-to-many mapping like let p = 0;
export { p as q, p as r }so this will be: const __vite_ssr_assign_p_ = (value) => {
__vite_ssr_exports__["q"] = p;
__vite_ssr_exports__["r"] = p;
return value;
} |
|
If we go with functions, maybe they can be generated per assignment/variable? If there is a single assignment, it can reuse it. If there are multiple assignments which is very rare, it can generate a new one: // output
const __vite_ssr_assign_x_ = (value) => {
__vite_ssr_exports__["x"] = x;
return value;
}
const __vite_ssr_assign_xp_ = (value) => {
__vite_ssr_exports__["x"] = x;
__vite_ssr_exports__["q"] = p;
return value;
}
someFn(__vite_ssr_assign_x_(x = 1));
someFn(__vite_ssr_assign_xp_([x, p] = [1, 2])));Although if we do it like this, I don't see why we can't just inline them 🤷🏻 |
|
But honestly, before we even go with either approach, we need to figure out if the Proxy approach is good enough. I feel like losing 30% in benchmarks affects the execution too much, maybe? As a note, Vitest doesn't even seal the module, but this goes against the standard. Users will eat us alive if Vite allows it for performance, I feel like 😞 |
|
A note: with the |
Yeah, going against the spec for the sake of "step-into" and perf doesn't feel right. But just one more try, if we loosen esm alignment by replacing // input
export let x = 0;
x = 1
// output
let __vite_ssr_exports__ = {};
let x = 0;
Object.defineProperty(__vite_ssr_exports__, "x", { configurable: true, enumerable: true, value: x })
const __vite_ssr_assign_x_ = (value) => {
Object.defineProperty(__vite_ssr_exports__, "x", { configurable: true, enumerable: true, value: x })
return value;
}
__vite_ssr_assign_x_(x = 1);
Object.preventExtensions(x);
// after this, normal assignment still errors
// __vite_ssr_exports__.x = 5678
// __vite_ssr_exports__.p = 5678
// but there's a loop hole
// Object.defineProperty(__vite_ssr_exports__, "x", { ... }) |
|
I don’t see how this is different from sealing it, to be honest. There is still a way to override properties, and you can also delete them! Every approach has a downside, I guess we just have to pick the poison. |
I guess no. That feature is not exposed in Node API (I'm not sure which feature you are referring to, but probably) and other APIs exposed from Node does not have a stable ABI. That means we'll have to compile the lib for each Node version, otherwise the users has to compile it. Even if we compile it for each version, AFAIK there's no way to do optional dependencies based on Node version, so all versions will have to be downloaded (unless we do it on our own with |
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.
Debugger should stop at line 1 or 2, but instead it stops at 7.
Source map of the
math.tsin 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
importintoimport()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.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.1Used Package Manager
pnpm
Logs
No response
Validations
The text was updated successfully, but these errors were encountered: