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

Native support for esm clients #4788

Closed
plastikfan opened this issue Jan 21, 2022 · 6 comments
Closed

Native support for esm clients #4788

plastikfan opened this issue Jan 21, 2022 · 6 comments

Comments

@plastikfan
Copy link

Attempting to import from express from an esm package, does not currently work:

import express from "express";

Taking a look at express's root index.js reveals:

module.exports = require('./lib/express');

this is not compatible with esm clients. The client needs to use the esm require workaround, but the user may not be aware of what this is. Eg, I am currently using rollup/typescript in an esm module and attempted to import express and was faced with a confusing error message that did not accurately reflect the problem at hand:

[16:21:47] Error: Unexpected token (Note that you need @rollup/plugin-json to import JSON files)
    at error (file:///home/plastikfan/dev/tutorials/auth0/auth0-node-express-tut/node_modules/rollup/dist/es/shared/rollup.js:10332:30)
    at Module.error (file:///home/plastikfan/dev/tutorials/auth0/auth0-node-express-tut/node_modules/rollup/dist/es/shared/rollup.js:12251:16)
    at Module.tryParse (file:///home/plastikfan/dev/tutorials/auth0/auth0-node-express-tut/node_modules/rollup/dist/es/shared/rollup.js:12654:25)
    at Module.setSource (file:///home/plastikfan/dev/tutorials/auth0/auth0-node-express-tut/node_modules/rollup/dist/es/shared/rollup.js:12557:24)
    at ModuleLoader.addModuleSource (file:///home/plastikfan/dev/tutorials/auth0/auth0-node-express-tut/node_modules/rollup/dist/es/shared/rollup.js:22021:20)

The user needs to esm require instead (but really ought not to):

import { createRequire } from "module";
const require = createRequire(import.meta.url);
const express = require("express");

Now that esm is becoming established it would be good for express to get onboard the esm bandwagon and natively support it for a better developer exprience.

@dougwilson
Copy link
Contributor

Hi @plastikfan sorry for any confusion. Our readme and website should have it documented how you import express, which is with require() and not import. I'm not sure where it would have indicated the use of import.

As far as adding it, I don't see any issue with adding suport, but I am not familiar with how one would do so. Would you be willing to make a pull request that adds the import in addition to our current require()?

@plastikfan
Copy link
Author

Hi @dougwilson, I do have some idea, let me investigate further, I would love the opportunity to contribute to this repo, cheers for now.

@Segmentational
Copy link

Hey, @plastikfan -- I'm not sure if using express@^5 is within capabilities for your project, but I made a little example with the following merge request.

@plastikfan
Copy link
Author

Thanks @Segmentational ill take a look

@plastikfan
Copy link
Author

plastikfan commented Feb 7, 2022

Actually I realised that I had done something wrong in my application using the rollup bundler, which was the real problem, so I am going to close this issue. The problem was caused by the bundling process of rollup. I am using rollup to bundle all code with the package, but not 3rd party dependencies. I forgot to add express to the "externals" entity in the rollup config, which is what caused the error I reported on originally. With the correct config, the normal import (import express from "express";) does indeed work. The error message in Rollup was not particularly helpful.

@GabenGar
Copy link

Yeah I remember using import express from "express" just fine on vanilla JS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants