sylvainpolletvillard / ObjectModel

Strong Dynamically Typed Object Modeling for JavaScript
http://objectmodel.js.org
MIT License
467 stars 30 forks source link

"main" entry point should be CommonJS module format #126

Closed kieranpotts closed 4 years ago

kieranpotts commented 4 years ago

Thanks for this fantastic library Sylvain.

I am writing my application in standard ES modules. My development server runs on the source code, transpilling dynamically at runtime.

npx cross-env NODE_ENV=dev NODE_PATH=./src/ \
  npx nodemon --watch ./src \
    --exec npx babel-node ./bin/http/dev --delay 3

A component of my application imports ObjectModel as follows.

import { ObjectModel } from 'objectmodel'

This causes the server to fail with the following error.

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /home/vagrant/app/src/node_modules/objectmodel/dist/object-model.js
require() of ES modules is not supported.
require() of /home/vagrant/app/src/node_modules/objectmodel/dist/object-model.js from /home/vagrant/app/src/domain/entities/abstract.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename object-model.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /home/vagrant/synced/src/node_modules/objectmodel/package.json.

I tried swapping out babel-node for esm to handle the runtime transpilation, but the result is the same.

On closer inspection, I think the problem lies in the ObjectModel package. The entry points to the package are defined in package.json as follows:

{
  "main": "dist/object-model.js",
  "module": "src/index.js",
}

Node.js expects the "main" entry point to adopt the CommonJS module format for imports, for legacy reasons, while the "module" entry point is for standard ES imports. However, the dist directory is built to the esm format:

// https://github.com/sylvainpolletvillard/ObjectModel/blob/master/rollup.config.js

export default {
    input: 'build/bundle-entry' + (isProduction ? '.js' : '.dev.js'),
    output: {
        file: 'dist/object-model' + (isProduction ? '.min.js' : '.js'),
        format: 'esm',
        sourcemap: true,
        extend: true
    },
    plugins: isProduction ? [terser()] : []
}

I think the output.format setting needs to be changed to cjs. 🤔

sylvainpolletvillard commented 4 years ago

Hello,

In previous versions, rollup was configured to produce UMD builds with both ESM and CJS support, but I changed it in v4 to only produce ESM builds. See v4 release notes for details

Node.js expects the "main" entry point to adopt the CommonJS module format for imports, for legacy reasons, while the "module" entry point is for standard ES imports.

That's not what I understand from Node.js docs :

Node.js will treat the following as ES modules when passed to node as the initial input, or when referenced by import statements within ES module code:

  • Files ending in .mjs.
  • Files ending in .js when the nearest parent package.json file contains a top-level field "type" with a value of "module".

Current package.json on this repo tries to follow the option 2.3 in this article: https://2ality.com/2019/10/hybrid-npm-packages.html#option-3%3A-bare-import-esm%2C-deep-import-commonjs-with-backward-compatibility ; except I don't provide a CommonJS build, because I think it's for the best for this library users to bring their own transpiler. I also explain my reasons in v4 release notes

So because package.json has the top field type: "module", Node should load it as ESM. I guess it is not working on Node versions <12, not supporting ESM natively. That's where you can bring your own transpiler, or polyfill such as https://www.npmjs.com/package/esm

But here there must be an issue with your transpiler configuration. The error message is pretty clear:

require() of ES modules is not supported. require() of /home/vagrant/app/src/node_modules/objectmodel/dist/object-model.js from /home/vagrant/app/src/domain/entities/abstract.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules. Instead rename object-model.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from home/vagrant/synced/src/node_modules/objectmodel/package.json.

That means the transpiler replace the import instruction by require, but without touching to the library code which is an ESM build, hence the error. I don't know your whole build step configuration, but I can show you how to build a CJS version of the lib if it helps ?

kieranpotts commented 4 years ago

Hi @sylvainpolletvillard

Thanks for your help. Yes, I'm using Babel for transpilation, and I'm not sure why it is trying to import ObjectModel as a CJS module (swapping my import for require()) when the package is configured to export only standard ES modules. I thought it was a mistake with the package configuration for ObjectModel, but on closer inspection it seems to be correct.

Anyway, I've worked around the problem by building my own CJS version of ObjectModel.

Thanks again for your help.

sylvainpolletvillard commented 4 years ago

No problem. I was expecting this move to ESM only to cause issues for some users with quite complex build steps or older versions of Node. Most lib authors are currently dealing with the problem by providing "hybrid" packages, but that just moves the complexity to our side and slows down the necessary transition to be able to just ship ESM everywhere. Hopefully, next Node LTS will ease this process for everyone.