expressjs
Add ES module option.
Adds an --esm switch with documentation in both the README and the app's usage message. When this switch is active, the bin/www (see below about the name of this file), app.js, and routes files all use ES module import/export syntax instead of Common JS require() syntax. Other small code modifications in support of this change are made in --esm mode, such as adding .js to some filename strings, defining a __dirname variable, and adding 'node:' to Node library references (I believe that this requires the end user to be running Node version 16 or greater, which hopefully isn't a problem; it's easy to back out the change if it is a problem). One change that affects the files generated in non-esm mode is that the generated package.json file now always includes a 'type' field that has value of either 'module' when the --esm switch is active or 'commonjs' by default.
The generator code structure has been modified by adding a template/mjs folder in parallel with the existing template/js folder. The mjs folder contains the ESM versions of the example route files. This means that any future changes to the example route files should be made in two places to maintain compatibility between the CJS and ESM. versions. An alternative to the parallel folder structure would be to generate the route files from ejs templates rather than to follow the existing approach of copying them verbatim. I went with the simpler existing approach under the assumption that the example route files will change very infrequently if at all.
Testing of --esm has been added to test/cmd.js. Some of this testing failed when attempting to load bin/www due to the lack of file extension. So, when --esm is active this file is named bin/www.js. The --esm tests are based on the (no-args) tests but do not repeat the directory-oriented tests:
I'm not sure why 13.x fails when 12.x succeeds, but I'm guessing that it has something to do with 13.x not having LTS. Anyway, I'm pretty sure that the 15.x failure is not related to anything in this PR, because I see the same Python "missing parentheses" error when I run tests on the master branch. For what it's worth, I think that the problem is that some of the Python code being run as part of the test is written in Python 2 syntax that is incompatible with Python 3 while the tests (at GitHub and on my machine) are being run using a Python 3 interpreter.
The documentation has been updated to state that Node 14.x or higher is required for the --esm switch, and the --esm switch should now lead to an error exit for all Node versions prior to 14, The only tests run for these earlier versions are to check for an error exit and for appropriate error messages. I believe that the esm automated tests will now pass for all Node versions, with versions 14 and higher running the 12 tests shown in the earlier image. However, testing with Node 15.x and higher still produces errors on my machine related to sass, but this is true on the master branch as well.
The files generated in --esm mode now use updated JavaScript syntax, with const replacing var and arrow functions replacing function expressions in callbacks.
Am I correct in thinking that all of the PR test failures also occur when testing the master branch, or do I need to do some more work on the PR related to automated tests?
Am I correct in thinking that all of the PR test failures also occur when testing the master branch, or do I need to do some more work on the PR related to automated tests?
I think that there are some PRs with solutions for the CI, let me check. Probably we should merge them first.
type module and the node: prefix are both not required, and type module specifically should be avoided. ESM should be .mjs.
type module and the node: prefix are both not required, and type module specifically should be avoided. ESM should be .mjs.
Agreed, neither is required. The point of the PR is to provide an option to those who would prefer to begin a new project as ESM and to use other newer Node/JavaScript features such as the node: prefix and ES6 syntax. The Node API docs going back to v16 show example code in both CJS and ESM syntax. It seems past time for express-generator to provide the same option.
The node: prefix works in CJS also, and is not required in either; I'm not sure why it would be useful to conflate that with "use ESM"; similarly, type module is harmful since it violates what .js means, so express should be actively pushing people to use .mjs for ESM, since it's the standard extension for that format.
@drjeffjackson do you have any plans to work on making the tests in the pipeline to pass?
@ljharb Thanks for the additional detail; that's helpful. Regarding your observation:
The
node:prefix works in CJS also, and is not required in either; I'm not sure why it would be useful to conflate that with "use ESM"
You're absolutely right. My original goal was to generate an ESM version of the skeleton code, but in implementing this I realized that this felt like a halfway measure and that what would be even better would be to generate an ES6 version of the code. Despite my development going in that direction, I still referred to this as an ESM mod. That was an oversight, and I appreciate your pointing this out. I'll be pushing a commit renaming the switch to --es6.
However, regarding your claim that:
similarly, type module is harmful since it violates what
.jsmeans, so express should be actively pushing people to use.mjsfor ESM, since it's the standard extension for that format.
The Node v21 Modules: Packages API says:
This flag [ --experimental-default-type ] currently defaults to "commonjs", but it may change in the future to default to "module". For this reason it is best to be explicit wherever possible; in particular, package authors should always include the "type" field in their package.json files, even in packages where all sources are CommonJS. Being explicit about the type of the package will future-proof the package in case the default type of Node.js ever changes, and it will also make things easier for build tools and loaders to determine how the files in the package should be interpreted.
There is no hint here or anywhere else in the Node docs, as far as I can tell, that type: 'module' is something to avoid. To the contrary, if there is a hint of anything it is that there is a real possibility that the default type of Node will change to module at some point in the future! There is also a relevant implicit assumption here that many of the files in a package, whether a package of type module or type commonjs, will have the .js extension: "Being explicit about the type of the package will ... make things easier ... to determine how the files in the package should be interpreted." If all of the files in a module type package had the .mjs extension, the extension by itself would determine how to interpret the files and the type field would be superfluous. So, the assumption of the quote is that the files matching the package type will likely have .js extensions and that it's therefore good practice to specify the package type, whatever it is.
In addition, a little farther down in the Modules: Packages page we find this:
The .mjs and .cjs extensions can be used to mix types within the same package:
- Within a "type": "module" package, Node.js can be instructed to interpret a particular file as CommonJS by naming it with a .cjs extension (since both .js and .mjs files are treated as ES modules within a "module" package).
- Within a "type": "commonjs" package, Node.js can be instructed to interpret a particular file as an ES module by naming it with an .mjs extension (since both .js and .cjs files are treated as CommonJS within a "commonjs" package).
Again, there's no hint here that anything is violated by using a "type": "module" package containing .js files that contain ESM code, any more than there is anything wrong with using a "type": "commonjs" package containing .js files that contain CJS code.
In short, I've looked, but I haven't found any support for your claim while I have found what seem to me to be strong indicators to the contrary. However, maybe I've missed something. I'd be interested in any pointers you have to reputable recent recommendations that "type": "module" be avoided in new projects.
@drjeffjackson do you have any plans to work on making the tests in the pipeline to pass?
@joeyguerra: I believe that my PR code passes all of the tests that I added for it and causes no problems with existing tests. That is, I believe that all of the test failures are due to other code that, as far as I can tell, also fails on the master branch. @UlisesGascon is I believe looking into fixing these tests, so no, I have no plans currently to correct these problems since they're not problems with my code/tests.
That said, if it is a problem for others to fix the failing tests and if all tests need to pass before my PR can be approved, it would not be hard for me to submit a separate PR that skips the failing tests on a version-specific basis, such as adding code that for v0.10 skips the pug tests that fail.
@drjeffjackson I pulled down the PR to investigate and found a bug in the
app.js.ejstemplate.createErrordoesn't exist in the--esm-> "should generate a 404" test case.createErroris only defined when--esmis not used. Adding the following causes the test to pass:<% if (esm) { -%> import express from 'express'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; <% Object.keys(modules).sort().forEach(function (variable) { -%> import <%- variable %> from '<%- modules[variable] %>'; <% }); -%> // The following line fixes the test. import createError from 'http-errors';
I am not sure what you are looking at, @joeyguerra? Here is what I see on my branch at GitHub:
@drjeffjackson you're correct. I had removed lines 2 through 4 when troubleshooting the "http-errors not found" issue in the node 15 pipeline run.
@drjeffjackson you're correct. I had removed lines 2 through 4 when troubleshooting the "http-errors not found" issue in the node 15 pipeline run.
Ah, okay. So the issue is in the master-branch code, which imports createError conditioned on view. My code does the same thing, and I think that you're saying that createError needs to be imported unconditionally in both versions. Anyway, sure, I can make that change in my code. Would you want me to change the original code to match as well?
@drjeffjackson I think the app.js.ejs code should "work" as it stands in this PR. I had deleted those lines as I was troubleshooting the missing http-errors module error, but forgot to put it back once I resolved it by installing Python 2.
@drjeffjackson the docs are written by folks with an agenda. I was on the modules working group, and i maintain module tooling, and I’m on the language committee, and i can assure you that .mjs is the standard extension for ESM; changing the default of the “type” field won’t affect anyone who uses .mjs - only those who use .js. It’s not that using type module “violates” anything, it’s that it’s sole purpose is to subvert why “.js” means - from CJS to ESM - and that can be bypassed simply by using the standard ESM file extension.
2 different viewpoints to the mjs debate.
For learning and portability purposes, we decided to keep to .js.
for Clartity for the reader and the runtime/tooling, use .mjs.
Btw, PR #314 adds Python 2 to the pipeline runner which I think would make all the pipeline tests in this PR pass.
@drjeffjackson the docs are written by folks with an agenda. I was on the modules working group, and i maintain module tooling, and I’m on the language committee, and i can assure you that .mjs is the standard extension for ESM; changing the default of the “type” field won’t affect anyone who uses .mjs - only those who use .js. It’s not that using type module “violates” anything, it’s that it’s sole purpose is to subvert why “.js” means - from CJS to ESM - and that can be bypassed simply by using the standard ESM file extension.
@ljharb Impressive resume, but I notice that you don't seem to have anything to point to to back up your claim about the type field other than your own informed opinion. I like to think that I have an impressive resume and informed opinions as well, and in my opinion new code should be written using ES6+, including ES modules. It's simply a better way to code. Also in my opinion, setting type to module is simply a way of encouraging that desirable behavior in new projects.
As for subverting the meaning of .js, .js means that a file contains JavaScript, pure and simple. Yes, if you look at old Node code, you are going to expect to see the CJS module system used in many .js files. But the question of interest is, what module system should we expect to see used in new Node .js files? I don't see any problem with saying that we don't know what to expect but that in a well-written package we will be able to look at package.json to find out. How is this subversive?
JavaScript has two parse goals, Script and Module, and since web browsers don't understand file extensions at all, the only runtime uses of .js before ESM was usable in any engine was the Script goal in browsers, and CJS in node. Additionally, CJS isn't just in "old" node code, it's the vast majority of npm's downloads.
The extension should be telling you what module system it is, and if you want to clearly indicate ESM, you'd use .mjs. You shouldn't have to look at package.json to know, and moving the file elsewhere shouldn't lose this out-of-band information when the proper in-band mechanism exists - the extension.
JavaScript has two parse goals, Script and Module, and since web browsers don't understand file extensions at all, the only runtime uses of
.jsbefore ESM was usable in any engine was the Script goal in browsers, and CJS in node. Additionally, CJS isn't just in "old" node code, it's the vast majority of npm's downloads.The extension should be telling you what module system it is, and if you want to clearly indicate ESM, you'd use
.mjs. You shouldn't have to look at package.json to know, and moving the file elsewhere shouldn't lose this out-of-band information when the proper in-band mechanism exists - the extension.
I agree that using .mjs has certain advantages. It also has some disadvantages, as noted in the MDN article.
btw i think "es6" isn't a good name; ES6 is ES2015, which was 9 years ago, and there's lots of module features added since then - and node didn't ship ESM support until 2020. if it's a broad "use one person's idea of modern practices" then a generic name like "modern" is probably more appropriate; if it's named something like "ESM" it should omit subjective opinions.
btw i think "es6" isn't a good name; ES6 is ES2015, which was 9 years ago, and there's lots of module features added since then - and node didn't ship ESM support until 2020. if it's a broad "use one person's idea of modern practices" then a generic name like "modern" is probably more appropriate; if it's named something like "ESM" it should omit subjective opinions.
@ljharb All of the changes to the generated code involve language features introduced by ES6: JavaScript modules, const, and arrow functions. So, --es6 is appropriate. Yes, the switch also assumes that new code written for the project will use the module syntax introduced by ES6 and sets the package type accordingly. That seems more appropriate to me than assuming otherwise, which would be the implicit assumption were the package type not specified.
I tested this with the following steps (pulled @drjeffjackson repo and esm branch)
npm install
npm link
express myapp --es6
cd myapp
npm install
DEBUG=myapp:* npm start
Visited http://localhost:3000 in my browser.
- Page was loaded as expected
- Looked in myapp and saw ES6 style javascript files and code
everything LGTM. Let me know if there's anything else I should look for.
#welldone
I tested this with the following steps (pulled @drjeffjackson repo and
esmbranch) [...]
Thanks, @joeyguerra! One other test would be visiting http://localhost:3000/users, which should respond with the text "respond with a resource". And you should also see ES6 style in bin/www, routes/index, and routes/users.
Looks really promising. When is it going to merge onto the actual package?
