Bring your own node_modules for incremental Deno adoption #18967

rajsite · 2023-05-03T06:02:08Z

As a node developer I'm excited about incrementally introducing Deno to exisiting projects. If Deno allowed an operating mode where I can use it in an existing single npm module project or npm workspace monorepo with my exisiting npm workflows to manage my npm dependencies that would be a HUGE for incremental adoption.

I appreciate deno trying to solve the problems with npm, but right now lack of local file: support, workspace support, build reproducibility, etc. are just blocking me from exploring deno in most projects besides one-off tools.

I would love to have a deno --bring-your-own-node-modules flag that used the npm: import specifier and just resolved the npm package using the package resolution algorithm locally on disk without trying to fetch dependencies, create links / junctions, update lock files, etc.

To be straightforward, I think the permissions model is useful, and I want to use it as quickly as possible. Improving the npm tooling is great but not what attracts me to deno. The permissions model + sandbox, focus on web standards compatibility, @deno_land, are what I want to start using and sharing.

This would let me stick Deno in the thick of large projects by swapping out node tooling incrementally. I could migrate the scripts using the exisiting node_modules, package.json, package-lock.json, etc managed by npm itself for the whole workspace. Even makes it easier to just test deno support with npm packages without introducing whole new workflows to patch npm packages locally, etc.

The text was updated successfully, but these errors were encountered:

dsherret · 2023-05-03T16:55:50Z

This is a nice idea, but it would be very challenging to do given Deno's current architecture. Deno works completely off of npm specifiers, and relies on knowing the npm resolution upfront. Ex. even if there's a package.json, that all gets transformed under the hood to npm specifiers. It would take a lot of effort to implement this and it might just be more worthwhile investing that into file:, workspace: etc. support.

Edit: Also, we would need to consider what happens when the user adds a package and doesn't then run npm install or some other tool.

rajsite · 2023-05-03T17:22:50Z

Deno works completely off of npm specifiers, and relies on knowing the npm resolution upfront

In this scenario wouldn't deno have that available based on the node_modules structure on disk? Alternatively, could even use the existing package-lock.json which has it all included.

It would take a lot of effort to implement this and it might just be more worthwhile investing that into file:, workspace: etc. support.

I don't think implementing those helps in the incremental adoption use-case as deno will mess with the local node_modules: #18803

It only works that way if deno owns the full workspace node_modules, etc. which will be a tougher sell to add to existing projects.

dsherret · 2023-05-03T17:28:18Z

In this scenario wouldn't deno have that available based on the node_modules structure on disk? Alternatively, could even use the existing package-lock.json which has it all included.

Deno doesn't understand how to read a node_modules folder to figure out resolution or to read an existing package-lock.json at the moment, which is why this would be a lot of work.

I don't think implementing those helps in the incremental adoption use-case as deno will mess with the local node_modules: #18803

It only works that way if deno owns the full workspace node_modules, etc. which will be a tougher sell to add to existing projects.

Ideally, the node_modules folder is unnecessary and already many packages work without it. We could also investigate only modifying the node_modules/.deno directory and not any top level node_modules/<package-name> folder if it already exists and is not a symlink/junction (edit: probably not. This could lead to subtle bugs). Deno essentially ignores the node_modules/<package-name> folder anyway.

…0718) This is required from BYONM (bring your own node_modules). Part of #18967

This makes `CliNpmResolver` a trait. The terminology used is: - **managed** - Deno manages the node_modules folder and does an auto-install (ex. `ManagedCliNpmResolver`) - **byonm** - "Bring your own node_modules" (ex. `ByonmCliNpmResolver`, which is in this PR, but unimplemented at the moment) Part of #18967

…the managed `CliNpmResolver` (#20777) Part of #18967

…0718) This is required from BYONM (bring your own node_modules). Part of #18967

This makes `CliNpmResolver` a trait. The terminology used is: - **managed** - Deno manages the node_modules folder and does an auto-install (ex. `ManagedCliNpmResolver`) - **byonm** - "Bring your own node_modules" (ex. `ByonmCliNpmResolver`, which is in this PR, but unimplemented at the moment) Part of #18967

…the managed `CliNpmResolver` (#20777) Part of #18967

dsherret · 2023-10-13T09:59:34Z

Just a note that I've been actively working on this the past few weeks.

Current thought on how this should work:

--node-modules-dir flag / "nodeModulesDir": true in deno.json - BYONM (bring your own node_modules). Default when a package.json is auto-discovered (BREAKING).
--node-modules-dir=managed / "nodeModulesDir": "managed" - Deno manages your node_modules folder and auto-installs packages (current --node-modules-dir behaviour and current default when a package.json is auto-discovered)
--node-modules-dir=false / "nodeModulesDir": false - Uses the global cache. Default when no package.json is auto-discovered (current behaviour)

So this would be a breaking change. We're thinking to land it even before Deno 2.0. Initially it will land under an unstable flag/config value.

wojpawlik · 2023-10-13T12:56:00Z

How about non-breaking "nodeModulesDir": "read-only" and "nodeModulesDir": "read-write"?

dsherret · 2023-10-13T12:59:00Z

It would be ideal for most Node projects to just work without additional setup instructions, so to achieve that it would have to be breaking.

rajsite · 2023-10-13T20:18:09Z

That sounds awesome! I'm excited about having that capability. I had one thought that could be interesting from a "deno runs your npm packages more securely" perspective.

It could be nice to have the ability to say what the top-level node_module directory allowed during npm module resolution is.

For example if I have:

projects/
    a/
        node_modules/
            foo
        package.json
        script.js
        sub/ 
            node_modules/
                 sub-foo
            package.json
            sub-script.js
node_modules/
    oops-foo

from projects/a/sub running: deno --node-modules-dir --node-modules-top-level=../ sub-script.js ie resolution is scoped to projects/a

The file sub-script.js could import sub-foo or foo but could not accidentally import oops-foo from outside what I consider the project directory of projects/a (and packages couldn't try and import from other places outside of the project folder I don't expect).

I'll admit it's a not a problem I've run into in practice (that I've noticed) but it's a quirk of node_module resolution I always found a bit odd.

) This PR adds a new unstable "bring your own node_modules" (BYONM) functionality currently behind a `--unstable-byonm` flag (`"unstable": ["byonm"]` in a deno.json). This enables users to run a separate install command (ex. `npm install`, `pnpm install`) then run `deno run main.ts` and Deno will respect the layout of the node_modules directory as setup by the separate install command. It also works with npm/yarn/pnpm workspaces. For this PR, the behaviour is opted into by specifying `--unstable-byonm`/`"unstable": ["byonm"]`, but in the future we may make this the default behaviour as outlined in #18967 (comment) This is an extremely rough initial implementation. Errors are terrible in this and the LSP requires frequent restarts. Improvements will be done in follow up PRs.

mlwilkerson · 2023-12-05T01:21:12Z

@dsherret I'm finding "Bring your own node_modules" to be valuable for another reason:

mixing public and private NPM registries by scope

My company publishes some packages only via private registry, which requires authentication. So we instruct customers to set up an .npmrc configuration file that associates the @fortawesome scope with npm.fontawesome.com and their authToken.

Under that configuration, npm install retrieves any packages that aren't @fortawesome from the default public registry, while retrieving all @fortawesome packages from npm.fontawesome.com, with authentication.

I saw in Deno's configuration documentation (here and here) that it's possible to change the registry that is used for retrieving packages (and, if memory serves, add authentication). And that worked--but it resulted in all packages being resolved through that registry, which is not workable.

So I wondered if, in Deno's progress toward deeper NPM compatibility, it might eventually provide per-scope registry configuration, like .npmrc.

However, now seeing this "Bring your own node_modules", it seems like this would eliminate the need for per-scope registry config. npm could populate node_modules and Deno could just use it, never having to deal with multiple NPM registries---auth for some, not for others, etc.

gplanansky · 2024-01-07T10:13:00Z

I get the error below importing Plotly.js-dist. The Cache content is the same as in an npm node_modules dir with a project that works. (The deno pl import works fine as does d3).

Is there an invocation to make it work? Or is it a current deno limitation?

If a limitation, please add Plotly.js to the list. Much used in data science, e.g. with pandas and polars.

$ deno
Deno 1.39.1
exit using ctrl+d, ctrl+c, or close()
REPL is running with all permissions allowed.
To specify permissions, run `deno repl` with allow flags.

> import * as pl from 'npm:nodejs-polars';
undefined

> import Plotly from "npm:plotly.js-dist@2.27.1"
Uncaught SyntaxError: Invalid or unexpected token at file:///Users/george/Library/Caches/deno/npm/registry.npmjs.org/plotly.js-dist/2.27.1/plotly.js:929:14
    at async <anonymous>:1:52

dsherret · 2024-01-07T20:25:56Z

@gplanansky see #21836

lino-levan · 2024-03-07T16:17:40Z

@dsherret with --unstable-byonm landing, is this still relevant?

gplanansky · 2024-03-09T14:58:44Z

I am still unable to import plotly.js in Deno. I believe I know why.
Plotly provides a cdn etc library for browser apps, which can use import syntax to import that library.

index.html:

...
<script type="module"> import "./myplotly.js" </script>
...

myplotly.js:

import "https://cdn.plot.ly/plotly-2.30.0.js"
...

The above works fine.

However the plotly.js node import needs a subscription token, without which npm imports fail.
Trying to import the cdn library also fails.

$ deno

Deno 1.41.2
exit using ctrl+d, ctrl+c, or close()
REPL is running with all permissions allowed.
To specify permissions, run `deno repl` with allow flags.
> import Plotly from "npm:plotly.js-dist@2.30.0"
Uncaught TypeError: Cannot read properties of undefined (reading 'navigator')
    at Object.__webpack_modules__.3400.lib.getFirefoxVersion (file:///Users/george/Library/Caches/deno/npm/registry.npmjs.org/plotly.js-dist/2.30.0/plotly.js:27079:49)

> import "https://cdn.plot.ly/plotly-2.30.0.js"
Uncaught ReferenceError: document is not defined
    at addRelatedStyleRule (https://cdn.plot.ly/plotly-2.30.0.js:25175:15)
    at Object.addStyleRule (https://cdn.plot.ly/plotly-2.30.0.js:25166:3)
    at Object.79288 (https://cdn.plot.ly/plotly-2.30.0.js:82:7)
    at __webpack_require__ (https://cdn.plot.ly/plotly-2.30.0.js:225942:42)
    at Object.64884 (https://cdn.plot.ly/plotly-2.30.0.js:23387:1)
    at __webpack_require__ (https://cdn.plot.ly/plotly-2.30.0.js:225942:42)
    at Object.32016 (https://cdn.plot.ly/plotly-2.30.0.js:213:18)
    at __webpack_require__ (https://cdn.plot.ly/plotly-2.30.0.js:225942:42)
    at Object.13792 (https://cdn.plot.ly/plotly-2.30.0.js:343:14)
    at __webpack_require__ (https://cdn.plot.ly/plotly-2.30.0.js:225942:42)

> const { default: plotly} = await import(import "https://cdn.plot.ly/plotly-2.30.0.js");
parse error: Unexpected token `string literal (https://cdn.plot.ly/plotly-2.30.0.js, "https://cdn.plot.ly/plotly-2.30.0.js")`. Expected `.` or `(` at 1:48

Question -- how to use the cdn library, in Deno? Deno being a Jupyter kernel, it would be nice to use the js plotly library alongside the js polars etc libraries in a notebook.

bartlomieju · 2024-03-31T05:54:43Z

This issue might be superseded by #23151

rajsite mentioned this issue May 3, 2023

Consider not using node_modules directory whenever node_modules dir exists and it does not contain a .deno directory #18803

Closed

dsherret added suggestion suggestions for new features (yet to be agreed) node compat labels May 3, 2023

dsherret mentioned this issue Jul 12, 2023

Deno should use the current node_modules folder structure instead of making its own .deno folder inside node_modules #19807

Closed

dsherret added the maybe 2.0 label Sep 13, 2023

rajsite mentioned this issue Sep 14, 2023

Deno support pyodide/pyodide#3420

Open

6 tasks

dsherret mentioned this issue Sep 27, 2023

refactor(ext/node): remove dependency on deno_npm and deno_semver #20718

Merged

dsherret added a commit that referenced this issue Sep 28, 2023

refactor(ext/node): remove dependency on deno_npm and deno_semver (#2…

d43e48c

…0718) This is required from BYONM (bring your own node_modules). Part of #18967

dsherret mentioned this issue Sep 28, 2023

refactor(cli): make CliNpmResolver a trait #20732

Merged

dsherret added a commit that referenced this issue Oct 3, 2023

refactor(npm): break up NpmModuleLoader and move more methods into …

8c1677e

…the managed `CliNpmResolver` (#20777) Part of #18967

bartlomieju mentioned this issue Oct 9, 2023

☂️ Node/npm compatibility issue for Q4 2023 #20851

Closed

4 tasks

bartlomieju pushed a commit that referenced this issue Oct 12, 2023

refactor(ext/node): remove dependency on deno_npm and deno_semver (#2…

8d851c9

…0718) This is required from BYONM (bring your own node_modules). Part of #18967

bartlomieju pushed a commit that referenced this issue Oct 12, 2023

refactor(npm): break up NpmModuleLoader and move more methods into …

d97701c

…the managed `CliNpmResolver` (#20777) Part of #18967

dsherret mentioned this issue Oct 24, 2023

feat(unstable): ability to npm install then deno run main.ts #20967

Merged

dsherret mentioned this issue Oct 26, 2023

fix(unstable/byonm): improve error messages #20987

Merged

dsherret closed this as completed Mar 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bring your own node_modules for incremental Deno adoption #18967

Bring your own node_modules for incremental Deno adoption #18967

rajsite commented May 3, 2023

dsherret commented May 3, 2023 •

edited

Loading

rajsite commented May 3, 2023

dsherret commented May 3, 2023 •

edited

Loading

dsherret commented Oct 13, 2023 •

edited

Loading

wojpawlik commented Oct 13, 2023

dsherret commented Oct 13, 2023

rajsite commented Oct 13, 2023 •

edited

Loading

mlwilkerson commented Dec 5, 2023

gplanansky commented Jan 7, 2024

dsherret commented Jan 7, 2024

lino-levan commented Mar 7, 2024

gplanansky commented Mar 9, 2024 •

edited

Loading

bartlomieju commented Mar 31, 2024

Bring your own node_modules for incremental Deno adoption #18967

Bring your own node_modules for incremental Deno adoption #18967

Comments

rajsite commented May 3, 2023

dsherret commented May 3, 2023 • edited Loading

rajsite commented May 3, 2023

dsherret commented May 3, 2023 • edited Loading

dsherret commented Oct 13, 2023 • edited Loading

wojpawlik commented Oct 13, 2023

dsherret commented Oct 13, 2023

rajsite commented Oct 13, 2023 • edited Loading

mlwilkerson commented Dec 5, 2023

gplanansky commented Jan 7, 2024

dsherret commented Jan 7, 2024

lino-levan commented Mar 7, 2024

gplanansky commented Mar 9, 2024 • edited Loading

bartlomieju commented Mar 31, 2024

dsherret commented May 3, 2023 •

edited

Loading

dsherret commented May 3, 2023 •

edited

Loading

dsherret commented Oct 13, 2023 •

edited

Loading

rajsite commented Oct 13, 2023 •

edited

Loading

gplanansky commented Mar 9, 2024 •

edited

Loading