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

Support Jiti (unjs) #20607

Closed
birkskyum opened this issue Sep 21, 2023 · 9 comments · Fixed by #23202
Closed

Support Jiti (unjs) #20607

birkskyum opened this issue Sep 21, 2023 · 9 comments · Fixed by #23202
Assignees
Labels
bug Something isn't working correctly node compat

Comments

@birkskyum
Copy link
Contributor

birkskyum commented Sep 21, 2023

Jiti - GitHub

Platform

Darwin 22.6.0 arm64 arm

Version

deno 1.37.0

Repro

// import console from "node:console";
let sum = 2+2;
console.log(`2+2 = ${sum}`);
console.log("Print this");

Expected

This output:

2+2 = 4
Print this

Actual

➜ deno run -A npm:jiti index.ts
error: Uncaught TypeError: Cannot read properties of undefined (reading 'log')
    at data::3:9
    at data::5:4
    at evalModule (file:///Users/admin/Library/Caches/deno/npm/registry.npmjs.org/jiti/1.19.3/dist/jiti.js:1:255456)
    at jiti (file:///Users/admin/Library/Caches/deno/npm/registry.npmjs.org/jiti/1.19.3/dist/jiti.js:1:253384)
    at Object.<anonymous> (file:///Users/admin/Library/Caches/deno/npm/registry.npmjs.org/jiti/1.19.3/bin/jiti.js:15:1)
    at Object.<anonymous> (file:///Users/admin/Library/Caches/deno/npm/registry.npmjs.org/jiti/1.19.3/bin/jiti.js:17:4)
    at Module._compile (node:module:733:34)
    at Object.Module._extensions..js (node:module:747:10)
    at Module.load (node:module:658:32)
    at Function.Module._load (node:module:539:12)
    at Object.loadCjsModule (ext:deno_node/02_init.js:52:22)
    at loadCjsModule ([ext:ext/node/lib.rs:595:29]:2:32)
    at [ext:ext/node/lib.rs:595:29]:3:7

Additional info

This works if importing node:console

@birkskyum birkskyum changed the title Support Jiti Support Jiti (unjs) Sep 21, 2023
@bartlomieju bartlomieju added bug Something isn't working correctly node compat labels Sep 22, 2023
@bartlomieju
Copy link
Member

bartlomieju commented Oct 12, 2023

Did some digging into it - what Jiti is doing is wrapping the provided code using Module.wrap and evaluating it using Script.runInThisContext, resulting in:

(function (exports, require, module, __filename, __dirname, Buffer, clearImmediate, clearInterval, clearTimeout, console, global, process, setImmediate, setInterval, setTimeout, performance) { (function (exports, require, module, __filename, __dirname) {"use strict";
let sum = 2 + 2;
console.log(`2+2 = ${sum}`);
console.log("Print this"); /* v7-ff3385c374396619 */
}).call(this, exports, require, module, __filename, __dirname); })

The problem is that after this wrapping, this function is evaluated directly and that causes the wrapper to miss some variables (everything after Buffer). We need to explicitly provide these variables somehow. The problem is that it's not guaranteed that every call to Script.runInThisContext will use Module.wrap, so we can't unconditionally bind variables.

@birkskyum
Copy link
Contributor Author

As @pi0 mentioned here, it might be necessary with an adapter to get this working, since Deno doesn't support the .cjs and thus can't skip jiti entirely the same way Bun does.

@bartlomieju
Copy link
Member

That's for floating the link @birkskyum, I will try to fix it (need to refresh my memory on the details of vm module); maybe we don't need to change anything in jiti after all. We have mechanisms to execute CommonJS, it's just they are not exposed to user code (but npm packages can use these mechanisms).

@birkskyum
Copy link
Contributor Author

Ah, interesting. If possible, that does sound like the better approach, because I doubt people would adopt Deno for Nuxt development if the transformer solution made the dx slower.

@pi0
Copy link

pi0 commented Oct 13, 2023

Thanks for following up on this @bartlomieju @birkskyum ❤️ Please let me know if could help on this (i am reachable via pi0 in discord as well).

@birkskyum
Copy link
Contributor Author

birkskyum commented Apr 9, 2024

@littledivy , just tried this with the latest canary (fad12b7). I still see the same error:

error: Uncaught TypeError: Cannot read properties of undefined (reading 'log')

@bartlomieju bartlomieju reopened this Apr 9, 2024
satyarohith pushed a commit that referenced this issue Apr 11, 2024
Implement contextified objects in `node:vm`

Fixes #23186
Fixes #22395
Fixes #20607
Fixes #18299
Fixes #19395
Fixes #18315
Fixes #18319
Fixes #23183
littledivy added a commit to littledivy/deno that referenced this issue Apr 19, 2024
@birkskyum
Copy link
Contributor Author

birkskyum commented Aug 6, 2024

I was curious to see if BYONM helped here, so I tried running this with latest deno 1.45.5 and DENO_FUTURE=1 deno task dev which executed jiti index.ts in package.json, but it gives the same output.

@birkskyum
Copy link
Contributor Author

This appear to work with the deno 2.0.0-rc.2+aaf2bf4

@pi0
Copy link

pi0 commented Sep 18, 2024

Right before jiti v2 release, we will use a fast path for Deno (similar to Bun), still impressive how Deno Node.js compatibility had been improved. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node compat
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants