Add ES module option.

[drjeffjackson]

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:

[drjeffjackson]

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.

[drjeffjackson]

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.

[drjeffjackson]

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?

[UlisesGascon]

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.

[ljharb]

type module and the node: prefix are both not required, and type module specifically should be avoided. ESM should be .mjs.

[drjeffjackson]

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.

[ljharb]

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.

[joeyguerra]

@drjeffjackson do you have any plans to work on making the tests in the pipeline to pass?

[drjeffjackson]

@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 .js means, so express should be actively pushing people to use .mjs for 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]

@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.

[joeyguerra]

Having completely separate files/templates for the ES6 style is easier for me to understand quickly. What are you thoughts about using .mjs file extension and creating a different .mjs template? That would remove the need for conditional logic in the app.js.ejs file. If not the mjs file extension, something else in the filename or folder to distinguish it from the Common JS style?

[drjeffjackson]

Having completely separate files/templates for the ES6 style is easier for me to understand quickly. What are you thoughts about using .mjs file extension and creating a different .mjs template? That would remove the need for conditional logic in the app.js.ejs file.

Actually, I had it that way on an earlier commit. But I thought that having to maintain parallel files might be a problem, so I went with the merged version. If you prefer parallel files, I'll change back.

[joeyguerra]

I assumed that the benefit for having separate files or even a separate folder of files only for the es6 templates would make the code easier to understand. "easier" here being "less conditionals to read in an ejs file without the benefit for code formatting helping the eyes follow the code".

What were your observations related to code readability when you had them separate?

[drjeffjackson]

I'd say that it was definitely more readable. The only issue would be, anyone maintaining this would have to remember to make any changes to app.js.ejs in two places, and there would be greater chance of introducing errors.
Also, what about www? I had that in two places originally.
As for the approach, I had added app.js.ejs and www.js.ejs to the mjs folder that this PR adds to the file structure.

[joeyguerra]

So the tradeoff would be improving code readability vs reducing Express app template maintenance effort. Both of these are for the future developer (which could be any of us ;)

I'm guessing there would have to be an API change in Express to cause a need to change the templates. That possibility might be pretty high given that development activity on Express is going up right now.

On the other hand, this feature (--es6) is essentially a different way to write an Express app. Other than the module loading syntax, the differences between the 2 styles can be opaque, but are there none the less. I would argue that having 2 different sets of templates increases the visibility of the design, which in theory, could reduce maintenance effort.

Ultimately, it's your call. I just wanted to put my 2 cents out there.

[drjeffjackson]

My latest push changes to parallel versions of app and www.

[drjeffjackson]

@drjeffjackson I pulled down the PR to investigate and found a bug in the app.js.ejs template. createError doesn't exist in the --esm -> "should generate a 404" test case. createError is only defined when --esm is 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:

[joeyguerra]

@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]

@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?

[joeyguerra]

using require("url") fixes the failing should export an express app from app.js test case.

[drjeffjackson]

I'm confused. When the tests were previously available here in the PR, this test was not failing. On which version is it failing now? Edit: Oh, v15, I see now.

[joeyguerra]

Yes, Node v15. I've been focusing on getting the tests to pass for Node 15 just because that's the one that's failing the pipeline, since this change is for node 14 and above.

I don't know why eval can't find node:url. Would love to know.

[drjeffjackson]

I'm guessing that 14 works because it was LTS and 15 does not work because it was not LTS. I ran into the same thing between LTS v12, which worked, and non-LTS v13, which did not.

[drjeffjackson]

Thanks for finding this issue! It's fixed in my latest push.

[joeyguerra]

@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.

[ljharb]

@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.

[joeyguerra]

2 different viewpoints to the mjs debate.

mdn

For learning and portability purposes, we decided to keep to .js.

V8 team

for Clartity for the reader and the runtime/tooling, use .mjs.

[joeyguerra]

Btw, PR #314 adds Python 2 to the pipeline runner which I think would make all the pipeline tests in this PR pass.

[joeyguerra]

Is this still required?

[drjeffjackson]

As I quoted earlier, it's recommended in the Node docs to explicitly set type for every package. So, no, it's not required, but I agree with the thinking that it should be included.

[joeyguerra]

Have you considered using a conditional and adding the type property instead of defaulting to commonjs ? The current code doesn't set a type property and defaulting to commonjs has a potential to change behavior for existing code.

  var pkg = {
    name: name,
    version: '0.0.0',
    private: true,
    scripts: {
      start: options.esm ? 'node ./bin/www.js' : 'node ./bin/www'
    },
    dependencies: {
      debug: '~2.6.9',
      express: '~4.17.1'
    }
  }
  if (options.esm) {
    pkg.type = 'module'
  }

[drjeffjackson]

Yes, I considered not setting type: commonjs explicitly. I'm almost positive that this will not change existing code. For one thing, express-generator is generating new code ;-) For another, even if someone dropped existing code underneath the generated code, the behavior of the existing code should not change since commonjs is the default for type. All this line does is make this default explicit and "future-proof" the package against possible later changes to the default. That's a good thing, I think.

[joeyguerra]

I see your point about commonjs being the default module loading style for Node. I’m just going to call this out as a risky change because it’s technically impacting existing functionality and can be written in a way to avoid that risk. Ultimately, only time can tell what the positive move is.

[drjeffjackson]

✔️ Fair enough.

[ljharb]

node isn’t going to be able to change the default; it’d break the world.

[drjeffjackson]

I wouldn't be too sure of that. After all, many folks thought that Y2K bugs might literally break the world, but they didn't.

[joeyguerra]

LOL. I heard someone say that Y2K bugs didn't break the world because of all the planning and work to prepare for it.

[drjeffjackson]

True enough. But in the case of changing the type default, the work for an individual package is adding one line to package.json if it's not already there. One could even imagine automated fixes for this change as opposed to bringing COBOL programmers out of retirement ;-)

[drjeffjackson]

All that said, I've made the change you recommended, @joeyguerra. The type field is now only generated when the --es6 switch is active.

[drjeffjackson]

@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?

[ljharb]

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.

[drjeffjackson]

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.

I agree that using .mjs has certain advantages. It also has some disadvantages, as noted in the MDN article. If the module type can stay in, I can generate files with .mjs extensions. My latest push implements this change.

[ljharb]

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.

[drjeffjackson]

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.

[joeyguerra]

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

[drjeffjackson]

I tested this with the following steps (pulled @drjeffjackson repo and esm branch) [...]

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.