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 npm URLs #13703

Closed
ry opened this issue Feb 17, 2022 · 24 comments
Closed

Support npm URLs #13703

ry opened this issue Feb 17, 2022 · 24 comments
Assignees
Labels
suggestion suggestions for new features (yet to be agreed)

Comments

@ry
Copy link
Member

ry commented Feb 17, 2022

Example:

import express from "npm:express@4";

const app = express();
const port = 3000;

app.get('/', (req, res) => {
  res.send('Hello World!');
});

app.listen(port, () => {
  console.log(`Example app listening on port ${port}`);
});

Depends on #12648

@ry ry added the suggestion suggestions for new features (yet to be agreed) label Feb 17, 2022
@lucsoft
Copy link

lucsoft commented Feb 17, 2022

Why have an built-in import-map?

{
    "imports": {
        "npm:": "https://unpkg.com/"
    }
}

this already solves the problem?

@kitsonk
Copy link
Contributor

kitsonk commented Feb 17, 2022

I am not supportive of this suggestion, as well as I think there are lots of semantics that would need to be defined to even really consider it.

Reasons I am not supportive:

  • Special sigils in specifiers are a problem. Node.js has implemented node: for built-ins and get around the security risk that built-in shadowing causes. TC39 has debated builtin modules and sigils in specifiers for long time with no conclusion on how to specify them. I am not sure we would just declare it as a special protocol.
  • This totally makes the code not portable to other platforms. Currently to get bare specifiers to work (in non-compat mode), users have to define an import map, and by defining an import map, they effectively keep it isomorphic with browsers and eventually Deno Deploy. We already have custom resolution logic when it comes to TypeScript type definitions which makes Deno TypeScript code incompatible with tsc, adding custom resolution logic baked into the runtime for runtime code seems like a really bad idea to me. No one else would be able to run that code.
  • While this might seem like a productive suggestion, it would require users to change or adapt any code. What happens with the code that is loaded that doesn't include the npm: in loading a dependency, how is that resolved?
  • How would this work with Deno Deploy?
  • I think there are big questions about even if it is viable to turn --compat mode on by default, which this suggestion implies. We do not yet have a clear story on how to mix Deno-first code with Node.js in compat mode, let alone having to deal with all the other eco-system problems that are present in npm/Node.js.
  • What about remote dependencies that contain these specifiers, do I suddenly include a 3rd party library and end up with a node_modules folder magically appearing? Something like this would at minimum need to be an opt-in, but I can't see a way of implementing it where it isn't an opt in, unless we turn it off for remote dependencies, but then that seems totally limiting for the feature as a whole.
  • The Node.js/npm ecosystem with CommonJS and ESM is a bit of a dumpster fire to put it kindly. Shoehorning a Deno CLI only, possibly not forward compatible way to access that eco-systems does not seem logical to me.
  • This doesn't address "which" npm registry will be used. In corporate environments it is often common to use a "private" npm registry to provide allow/block list capability as well as ensure security and compliance.
  • Interactions with deno compile and deno bundle are undefined at the moment. It is logical that this will cause issues.
  • So while aspects of this might be opt-in, a feature like this doesn't go away, is likely to be littered with bugs that the core team would have to address and distracts us from working on other things.

The current widely used solution in Deno to access npm packages, is to use a CDN like esm.sh, esm.run and skypack.dev. Admittedly there are some rough edges and the usability and something we need to work on, but it helps ensure that Deno first and npm code work together without having to bring the whole of the npm/Node.js ecosystem with it. While esm.sh is great, there are lots of improvements that could be made with something like npm.deno.land that helps ensure a smooth frictionless journey and helps ensure people can move smoothly from a legacy Node.js world to a more modern runtime.

While there are some challenges and limitations with import maps, they are a) a web standard, b) provide transparent and user editable configuration. Being transparent on resolution and auditable is a big thing versus some sort of opaque mechanism, because we are likely to get it wrong sometimes, and a user editing a bit in an import map (or having better and richer tools to manipulate it) is a lot better than something locked away in the binary you can't influence.

@elycheikhsmail
Copy link

Why have an built-in import-map?

{
    "imports": {
        "npm:": "https://unpkg.com/"
    }
}

this already solves the problem?

But how --compat will know this is code writen for node ? I suggest to have convention about npm import file config like deno.json convention

@ry
Copy link
Member Author

ry commented Feb 18, 2022

@kitsonk

Special sigils in specifiers are a problem. Node.js has implemented node: for built-ins and get around the security risk that built-in shadowing causes. TC39 has debated builtin modules and sigils in specifiers for long time with no conclusion on how to specify them. I am not sure we would just declare it as a special protocol.

The npm: specifiers are simply another URL. This does not violate any standard.

This totally makes the code not portable to other platforms. Currently to get bare specifiers to work (in non-compat mode), users have to define an import map, and by defining an import map, they effectively keep it isomorphic with browsers and eventually Deno Deploy. We already have custom resolution logic when it comes to TypeScript type definitions which makes Deno TypeScript code incompatible with tsc, adding custom resolution logic baked into the runtime for runtime code seems like a really bad idea to me. No one else would be able to run that code.

Deno code is not portable to other platforms. Deno code is generally Deno-specific typescript (with extension imports) - so does not work directly in browsers, nor directly with canonical TypeScript.

While this might seem like a productive suggestion, it would require users to change or adapt any code. What happens with the code that is loaded that doesn't include the npm: in loading a dependency, how is that resolved?

The resolution algorithm will be changed inside of npm packages, so bare specifiers will be resolved via the node resolution algorithm. In normal Deno scripts, bare specifiers will work as they currently do.

How would this work with Deno Deploy?

Transparently - I see no difficulties here.

I think there are big questions about even if it is viable to turn --compat mode on by default, which this suggestion implies. We do not yet have a clear story on how to mix Deno-first code with Node.js in compat mode, let alone having to deal with all the other eco-system problems that are present in npm/Node.js.

This suggestion does not imply --compat by default.

What about remote dependencies that contain these specifiers, do I suddenly include a 3rd party library and end up with a node_modules folder magically appearing? Something like this would at minimum need to be an opt-in, but I can't see a way of implementing it where it isn't an opt in, unless we turn it off for remote dependencies, but then that seems totally limiting for the feature as a whole.

Probably remote dependencies with npm: specifers will work. When using an npm: specifier, the node_modules folder will not be created - the package instead is loaded into DENO_DIR, similar to how yarn2 works.

The Node.js/npm ecosystem with CommonJS and ESM is a bit of a dumpster fire to put it kindly. Shoehorning a Deno CLI only, possibly not forward compatible way to access that eco-systems does not seem logical to me.

You should read Strategy Letter II: Chicken and Egg Problems

This doesn't address "which" npm registry will be used. In corporate environments it is often common to use a "private" npm registry to provide allow/block list capability as well as ensure security and compliance.

It would probably use the default registry and maybe we'd add a NPM_REGISTRY env var that could be set.

Interactions with deno compile and deno bundle are undefined at the moment. It is logical that this will cause issues.

I'm confident they can be made to work.

So while aspects of this might be opt-in, a feature like this doesn't go away, is likely to be littered with bugs that the core team would have to address and distracts us from working on other things.

It will not be littered with bugs - this is a straightforward feature. Most of the infrastructure is already in place.

@crowlKats
Copy link
Member

Deno code is not portable to other platforms. Deno code is generally Deno-specific typescript (with extension imports) - so does not work directly in browsers, nor directly with canonical TypeScript.

For dotland there is a requested feature and PR to transpile ts to js, so browsers can consume /x/ modules. This suggestion would quite hinder that (on top of the deno ns, but thats another topic)

@kt3k
Copy link
Member

kt3k commented Feb 23, 2022

I like this proposal better than the current --compat mode. This blends Deno ecosystem and npm ecosystem in a more reasonable way.

This doesn't expose fs or http Node.js modules to Deno-first module. So we can still write Deno code as it is, but also optionally use any npm modules. So this addresses the most frequent complaints about Deno (unavailability of npm) very elegantly in my view.

@lucsoft

Why have an built-in import-map?

This proposal suggests we switch the runtime to what the current --compat mode provides for npm modules. That means expressions like require("fs") works only inside of npm modules (npm modules works in a simulated node.js context)

@josephrocca
Copy link
Contributor

For dotland there is a requested feature and PR to transpile ts to js, so browsers can consume /x/ modules. This suggestion would quite hinder that (on top of the deno ns, but thats another topic)

@crowlKats I'm a huge fan of that proposal so I'm curious why this proposal would hinder it? I was under the impression that the all the node.js compatibility polyfills are in ts, so it would be able to run in browser after being compiled to js (assuming the npm module doesn't e.g. use file system / native addons / etc., but same can be said of Deno modules)? I.e. I'd have thought that the node stuff could essentially be "browserified". I'm probably misunderstanding something here.

@lucsoft
Copy link

lucsoft commented Feb 23, 2022

@kt3k okay i didn't know it switched to a nodejs context. i thought it just downloads it but this clarifies it for me. thanks! 😊

but why not allow something like this:

import express from "https://registry.npmjs.org/express/4.17.3";

const app = express();
const port = 3000;

app.get('/', (req, res) => {
  res.send('Hello World!');
});

app.listen(port, () => {
  console.log(`Example app listening on port ${port}`);
});

This could check the endpoint if its an npm like registry and provides everything in the background for you

@kt3k
Copy link
Member

kt3k commented Feb 24, 2022

@lucsoft

import express from "https://registry.npmjs.org/express/4.17.3";

Something like that might work, but registry.npmjs.org doesn't host script files directly. They provides .tar.gz files for each version of npm module. So we also need to change the resolution logic for those urls. That might look confusing to the users. npm: scheme would tell it more explicitly that we are doing something completely different from normal urls.

@josephrocca
Copy link
Contributor

This proposal suggests we switch the runtime to what the current --compat mode provides for npm modules. That means expressions like require("fs") works only inside of npm modules (npm modules works in a simulated node.js context)

@kt3k Why not just make npm accessible via something like https://deno.land/npm/package-name? That way the complexity is added to the building and serving of the Deno-compatible npm packages instead of adding extra complexity and browser-incompatibility to the Deno runtime itself.

Or is it not possible to solve this via some sort of transpilation/shimming of npm packages? That seems unlikely, but I might be misunderstanding something. If it is possible, then the cost of this npm: idea (specifically RE reducing browser compatibility) seems to far outweigh the benefits.

I think it'd be really bad idea to hurt browser compatibility here if there were some other viable way to get npm packages working in Deno.

@lucsoft
Copy link

lucsoft commented May 18, 2022

Why not just make npm accessible via something like https://deno.land/npm/package-name

We already have that its esm.sh/package-name

@ryanwhite04
Copy link

ryanwhite04 commented May 18, 2022

Why have an built-in import-map?

{
    "imports": {
        "npm:": "https://unpkg.com/"
    }
}

this already solves the problem?

But how --compat will know this is code writen for node ? I suggest to have convention about npm import file config like deno.json convention

By adding a query parameter of ?module to unpkg.com URLs, any bare import specifiers will automatically be expanded to absolute URLs, and it will server ES6 modules of all packages that have that specify a "module" in their package.json.

This way there doesn't need to be any extra configuration like a package.json or any non-web compatible changes to deno.

If a package isn't offereing an ES6 compatible version, usually there is already an issue on their github asking for it and it's only a matter of time.

I've been using this a lot, and it works well for most packages but requires suffixing all imported modules with ?module which is a little annoying.

Also, trying to importnpm:x.js?module with

{ "imports": { "npm:": "https://unpkg.com/" } }

on deno version 1.21.2
returns an error for me:

error: Unsupported scheme "npm" for module "npm:x.js". Supported schemes: [
    "data",
    "blob",
    "file",
    "http",
    "https",
]

but importing npm/x.js?module with

{ "imports": { "npm/": "https://unpkg.com/" } }

works fine.

If you don't want to suffix all of your imports with "?module" you have to include a separate mapping for every single package you want from npm/unpkg.com specifying the entire URL, so if you just want to import "npm/foo.js, You need

{ "imports": { "npm/foo.js": "https://unpkg.com/foo/bax/bar/foo.js?module" } }

It would be nice if importmaps passed on query parameters to redirects/mappings like

{ "imports": { "npm/": "https://unpkg.com/?module" } }

I saw some discussion about this on WICG/import-maps but currently trying this results in

Import map diagnostics:
  - Invalid target address "https://unpkg.com/?module" for package specifier "npm/". Package address targets must end with "/".
error: Blocked by null entry for ""npm/""

esm.sh works in a similar way, where the returned resource can be changed depending on the query parameter passed to it but with a lot more options.

@josephrocca
Copy link
Contributor

josephrocca commented May 18, 2022

Why not just make npm accessible via something like https://deno.land/npm/package-name

We already have that its esm.sh/package-name

@lucsoft I think this issue is about trying to avoid the need for the --compat flag rather than just transforming the imports. So the "shim" that I mentioned would need to be doing Deno-specific stuff, and hence esm.sh/unpkg.com/etc. wouldn't be enough. It needs to set Node globals, make built-in modules like fs importable, etc.

So, naively (not suggesting as actual solution), https://deno.land/npm/ would need to add something like Deno.enableNodeCompatibility() to the top of the script at the very least (as well as the import transformation stuff).

@nayeemrmn
Copy link
Collaborator

With regards to whether or not this requires --compat, implies --compat by default, or as described by @kt3k only applies it to npm modules; note that Node has globals like Buffer as well. Is the suggested behaviour to put these in the global scope for the whole program whenever there is an npm: module in the graph? If so, it's a bit strange for that not to require a flag even for transitive npm: dependencies.

I think we should start by allowing "compat": true in the config file, and evaluate if it's too much burden to gate this feature by.

@garretmh
Copy link

Regarding the scheme format, allowing specifying a (non-npm) authority would be pretty useful.
Example: npm:[//<authority>/]<module>[/<path>]

@jsejcksn
Copy link
Contributor

Regarding the scheme format, allowing specifying a (non-npm) authority would be pretty useful. Example: npm:[//<authority>/]<module>[/<path>]

@garretmh By "authority" do you mean registry? I can see the value in other registries (e.g. GitHub, private registries, etc.)

@garretmh
Copy link

@jsejcksn Essentially, yes. But technically I'm referring to a URI authority.

See https://developer.mozilla.org/en-US/docs/Learn/Common_questions/What_is_a_URL#basics_anatomy_of_a_url

However I don't think it would be feasible to support registry URLs that include a path section (e.g. "https://skimdb.npmjs.com/registry").

@josephrocca
Copy link
Contributor

josephrocca commented Aug 16, 2022

@kitsonk @ry @lucacasonato @crowlKats Can I ask a couple of questions about the direction that's being taken here according to the latest blog post? If so:

  1. With the chosen approach is there a way to create code that uses npm: modules, but works in the browser and in Deno? I'm hoping this is possible with import maps that change the "npm:" part to the unpkg base url or something like that? But then how would that work for dependencies (or dependencies of dependencies) that use npm:? I.e. how is browser-incompatibleness prevented from "infecting" all descendent modules? There are many npm modules that work fine in the browser, but (with my limited understanding of the details here) it seems like npm: would needlessly make dependent modules browser-incompatible?
  2. What is the technical reason that something like https://deno.land/npm/express@6 can't be used instead, where the deno.land cached version has been "fixed" to work in Deno (with some special init code added to the top of the script) only if it can't be "browserified"? That would seemingly maximise browser compatibility by only introducing Deno-specific/Node-specific stuff where it's actually needed.

My main worry here is that people will start using npm: very liberally, even when it's not necessary (i.e. they could have used an unpkg.com URL, for example), and so a bunch of packages (and, critically, all descendent packages) would be unnecessarily browser-incompatible, which goes against my main reason for using Deno in the first place, and why I'm so excited about it as a new JS runtime.

Sorry if these questions have been answered already in some other thread that I've missed. Thanks!

@jaydenseric
Copy link

Presumably a problem with the npm: idea is you would need to have a NODE_ENV environment variable populated in the Deno runtime environment set to development or production if you are importing things like npm:react-dom/server. With a CDN URL you can specify what dev or prod version of the module to load using a query string parameter like ?dev:

https://esm.sh/[email protected]/server?dev

React requires NODE_ENV to be production to opt-in to the production logic, or it falls back to the dev logic:

https://unpkg.com/[email protected]/server.node.js

Other packages have it inverted, where prod logic is the default.

Either way, npm: approach would result in both dev and prod logic being downloaded, cached, and parsed in some situations like this:

https://unpkg.com/[email protected]/client.js

That module is meant for client use, but it shows how packages use such logic with inline code instead of nested require calls.

Ideally over time npm packages are published as pure ESM, with dev and prod builds that can be selected via import maps and CDN's that don't do any build steps at all but just statically host the published files (like unpkg.com).

Imagined dev import map:

{
  "imports": {
    "react/": "https://esm.sh/[email protected]/dev/",
    "react-dom/": "https://unpkg.com/[email protected]/dev/",
  }
}

Imagined prod import map:

{
  "imports": {
    "react/": "https://esm.sh/[email protected]/",
    "react-dom/": "https://unpkg.com/[email protected]/",
  }
}

Imagined imports:

import createElement from "react/createElement.mjs";
import hydrateRoot from "react-dom/hydrateRoot.mjs";

There would be no main index modules to map, if packages had optimal JavaScript module design. No /server/ or /client/ subpaths.

Are projects like React so ossified, untouchable, and aloof that we would rather build non-standard features into JavaScript runtimes than simply fix the module format and structure of their packages so they align to web standards, allowing them to be used efficiently and universally with the tools we have today in Node.js, Deno, and browsers?

A JavaScript runtime should align to web standards, and not start cramming in proprietary features that will lead to a fractured ecosystem based on non web standard features that endorse arbitrary for-profit corporations. How would we feel if Chrome added support for an npm: URL scheme for JavaScript imports? It would rightfully get shot down in flames and never get past proposal stage.

We already have a solution for using legacy CJS npm packages in Deno and browsers until they clean up their act and publish ESM: CDNs like esm.sh. Why don't we just fix up the remaining bugs such CDNs have, and work with that? The engineering work to fix the CDN bugs is surely a lot less/cheaper than implementing the npm: scheme. Ideally we would have a highly reliable CDN maintained by Deno, e.g. https://deno.land/npm/[email protected]/server?dev; I wouldn't mind paying for such a CDN.

As other commenters have pointed out, the npm: approach won't work in browsers, so for legacy CJS npm packages like react we would have to find a CDN that provides them in ESM format for the browser runtime anyway. Almost all of my Deno projects are isomorphic, i.e. dependencies are used in both server and browser environments.

@dsherret
Copy link
Member

dsherret commented Aug 17, 2022

@josephrocca there are certain npm packages that can't work in the browser, but will eventually work in Deno.

For packages that are possible to transpile for the browser, the npm specifiers can be mapped to a transpile server like esm.sh. deno info --json will provide enough information that it will be possible to automatically generate an import map, that maps npm:<package-name>@<version-requirement> style imports, to import map entries like "npm:<package-name>@<version-requirement>": "https://esm.sh/<package-name>@<resolved-version>?deps=<resolved-deps-list-goes-here>". There's a proof of concept for this idea working in fresh here.

So, these specifiers provide a way for dependencies to state their version requirements, rather than depend on a single explicit version, then the runtime can do the work of automatically de-duplicating dependencies. For the browser, although this requires an extra step by using an import map, this can be auto-generated and you get the added benefit of probably less code delivered to the user because dependencies have started expressing version requirements instead.

There are several technical reasons why a transpile server like esm.sh doesn't always work. First, it's not always possible to convert cjs to esm. There can be stuff like synchronous deferred requires, or we've seen code that's non-analyzable using requires in an eval call. Additionally, there are sometimes data files in a package that are read synchronously and make including them in the graph difficult or cause some overhead. Sometimes there's also binary files in packages that need a way to be run synchronously, or with eventual support for Node API there could be extra files it uses in the package that would be impossible to statically analyze for.

@josephrocca
Copy link
Contributor

josephrocca commented Aug 17, 2022

@dsherret Thanks for your reply! So am I correct in understanding that library/package authors would ideally use unpkg.com/esm.sh/etc whenever possible (instead of npm:) so their package is automatically compatible with the browser without a build step?

If so, I'm wondering: Would it somehow be possible to give a warning if npm: is used when it's not actually necessary (i.e. compat mode isn't required for that particular npm package), or is there at least some other way to try to push authors away from making a module accidentally-incompatible-for-no-good-reason with build-step-less direct/simple import in the browser?

Or can we just solve this "browser-incompatible-for-no-good-reason" case with on-the-fly (and cached, of course) pre-compilation/transpilation when a deno.land/x module is downloaded to a browser, as is going to be done for .ts files anyway?

If that's the case, then that's somewhat relieving.

Thanks again for your reply!

@dsherret
Copy link
Member

dsherret commented Aug 17, 2022

If you have an import map that maps npm specifiers to something like unpkg, then this is not a build step. If a package ships with typescript, then that's browser incompatible and actually requires a build step—we don't have a warning for that so I don't think we would give a warning for this (though we'll discuss it internally).

Or can we just solve this "browser-incompatible-for-no-good-reason" case with on-the-fly (and cached, of course) pre-compilation/transpilation when a deno.land/x module is downloaded to a browser, denoland/website_feedback#19?

Yes, I think this is something that could be better solved by registries where if you specify you want a browser compatible module, then it will convert npm specifiers to an esm package, handle deduplication for you, and convert ts to js.

@dsherret
Copy link
Member

A very very rough unstable implementation of this landed in #15484. We're missing a lot of stuff like deno info/vendor support, lockfile, lsp, typescript, peer dependency support, but we'll follow up with other issues and work progressively on improving it.

@imjamesb

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests