denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
93.91k stars 5.22k forks source link

Enable Node interop against local modules #22655

Open ryanblock opened 6 months ago

ryanblock commented 6 months ago

What I'm trying to do

We're stoked to publish and contribute to the JSR ecosystem! While we generally keep our JS pretty close to the metal, as it were, some of our modules (like aws-lite) must unfortunately maintain CJS compatibility (I know...).

These projects, as one might expect, have an extensive existing test suites. It would be really helpful to enable Node interop mode when running those test suites locally. (I'm not sure, but this issue may be a variant of #15624 or #19621; the workaround I discovered / describe below here sounds very much like the one found here: https://github.com/denoland/deno/issues/15624#issuecomment-1511621310.)

What I've tried

After the removal of --compat, from what I can tell, the proper way to run a module with Node API / globals compatibility is with the npm: specifier. So far this has worked really well for us! Node compatibility mode is super polished, big ups for all the work there. However, it kind of breaks down when the Node module in question is a local project with unpublished changes. Let's jump into an example.

Say I have a CJS project named foobar, published to npm, and it looks like so:

├── src
│   └── index.cjs
├── test
│   └── test-suite.mjs
└── package.json

Here's how our tests would run locally in Node:

// test/test-suite.mjs
import test from 'tape'
import mod from '../src/index.cjs'

// Run the tests
test(t => t.ok(mod.method()))

Now let's say we want Deno to run the above test suite. Here's what happens:

// test/test-suite.mjs
import mod from '../src/index.cjs'
// error: Uncaught SyntaxError: The requested module '../src/index.cjs' does not provide an export named 'default'

// Long shot: try to put the relative file path in npm mode
import mod from 'npm:../src/index.cjs'
// error: Error getting response at https://registry.npmjs.org/ for package "..": missing field `name` at line 1 column 474
// A similar error occurs for the absolute file path

import mod from 'npm:foobar'
// This works! But it's running the already published version, not the local version we need to test

Additionally, I've tried forcing compatibility mode with a deno.json import map:

{
  "imports": {
    "npm:foobar": "./src/index.cjs"
  }
}
import mod from 'npm:foobar'
// error: Uncaught SyntaxError: The requested module 'npm:foobar' does not provide an export named 'default'

Current workaround

Because of the size of the test suites (sometimes many thousands of tests), it's not feasible for me to maintain a second Deno-specific test suite – and because of the CJS compatibility requirement, it wouldn't really work, either.

I did find a workaround after a number of hours of hacking around here, but it's pretty gnarly. I can get the existing test suites running with minimal code changes by taking the following steps:

This does actually work, but it feels wrong and brittle.

Ideal solution

I think my ideal solution here would be to allow npm: specifiers in import maps to put that particular dependency into Node compatibility mode. But I'm totally open to ideas.

Thanks for your consideration!

Metadata

Mac M2, Sonoma 14.3.1

deno --version
deno 1.41.0 (release, x86_64-apple-darwin)
v8 12.1.285.27
typescript 5.3.3
littledivy commented 6 months ago

Thanks for the detailed report. I think this is solved by the BYONM feature ("Bring your own node modules").

Create a deno.jsonc in the project root. This tells Deno to use the installed node_modules.

// deno.jsonc
{
  "unstable": ["byonm"],
}
littledivy commented 6 months ago

Ooops, I just realised this won't work because of CJS source. BYONM works with ESM modules at the moment.

ryanblock commented 6 months ago

@littledivy thanks, and correct me if I'm wrong, but per the docs, byonm relates to using and managing the node_modules folder. In this case I'm trying to ensure that tests will run correctly against the project itself, so even if there was an ESM entry or something, I don't think it would really matter? wdyt?

birkskyum commented 3 months ago

Wonder if the situation has bettered after this which is in 1.43.3

ryanblock commented 2 months ago

@birkskyum I'm not entirely sure of the nature of that merged PR, but running our test suite again with Deno 1.44.1 totally fails.

ryanblock commented 2 months ago

@littledivy any progress here?