meteor icon indicating copy to clipboard operation
meteor copied to clipboard

[BUG] [2.5.6] import of npm library gives empty object

Open trusktr opened this issue 3 years ago • 11 comments

Blocker: Unable to run a Meteor app because import of a library returns an empty object, as described in all these threads:

  • https://github.com/meteor/meteor/issues/10065
  • https://forums.meteor.com/t/importing-from-npm-returns-an-empty-object/44491
  • https://github.com/thereactivestack-legacy/meteor-webpack/issues/235
  • https://stackoverflow.com/questions/55382092/importing-gitlab-package-from-meteorjs-returns-empty-object
  • https://forums.meteor.com/t/solved-require-import-npm-require-all-return-empty-object/50153/6
  • https://ethereum.stackexchange.com/questions/60335/web3-empty-object-server-side-in-meteor
  • https://github.com/VulcanJS/Vulcan/issues/2580

I don't know if this ever worked in previous versions of Meteor, but from the threads we can see the issue celebrated quite a few birthdays already.

I think @benjamn might have the most clue as to what this might be caused by; he architected Meteor's module system.

Meteor 2.5.6 Linux Ubuntu 21.10 Node.js v17.3.1 npm 8.3.1

Reproduction:

git clone [email protected]:trusktr/mapapp.git
cd mapapp
git checkout meteor-issue-11900
npm install
meteor --exclude-archs "web.browser.legacy, web.cordova"

Error output:

W20220209-01:57:29.900(-8)? (STDERR) (node:28466) UnhandledPromiseRejectionWarning: TypeError: dayjs is not a function
W20220209-01:57:29.910(-8)? (STDERR)     at createFixtures (server/imports/fixtures.js:15:40)
W20220209-01:57:29.916(-8)? (STDERR)     at server/main.js:10:2
W20220209-01:57:29.920(-8)? (STDERR)     at /home/trusktr/.meteor/packages/promise/.0.12.0.1vb72xd.6v12++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/meteor-promise/fiber_pool.js:43:40
W20220209-01:57:29.921(-8)? (STDERR) (Use `node --trace-warnings ...` to show where the warning was created)
W20220209-01:57:29.922(-8)? (STDERR) (node:28466) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
W20220209-01:57:29.929(-8)? (STDERR) (node:28466) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

trusktr avatar Feb 09 '22 10:02 trusktr

Added a reproduction.

trusktr avatar Feb 09 '22 10:02 trusktr

@trusktr Awesome that you have a simple reproduction!

For the record, this seems to specifically affect ES Modules (Material UI in my case), and the server build.

I recently hit an unexpected issue recently when exploring ES modules, with NPM packages and Next.js: files with ".js" extensions maybe interpreted differently depending if "type": "module" is enabled or not.

  • If "type": "module" is enabled in package.json, ".js" is treated as ES modules. You have to name CommonJS ".cjs" ideally.
  • If not, eg when using conditional exports instead, ".js" is treated as CommonJS. You have to name ES modules ".mjs" ideally.

Maybe it's related? Like Meteor wrongly consider the package as CommonJS during the server build.

For example DayJS is not explicitely using "type":"module": https://github.com/iamkun/dayjs/blob/dev/package.json nor using ".mjs" extension.

For Material UI the package.json has:

"main": "./node/index.js",
  "module": "./index.js",

eric-burel avatar Feb 09 '22 10:02 eric-burel

Hi @trusktr, thank you for the detailed issue!

I saw that you have the package dayjs as a devDependencies.

I tested moving it to dependencies, then running rm -rf node_modules, then meteor npm install, and it was fixed.

Can you try that?

denihs avatar Feb 09 '22 14:02 denihs

Maybe it's related? Like Meteor wrongly consider the package as CommonJS during the server build.

Meteor doesn't understand Node.js ESM or package.json exports fields yet (https://github.com/meteor/meteor/issues/10906).

I saw that you have the package dayjs as a devDependencies.

I tested moving it to dependencies, then running rm -rf node_modules, then meteor npm install, and it was fixed.

Ah, moving it from devDependencies to dependencies worked. I didn't remove node_modules ore reinstall, I only moved the entry from devDependencies to dependencies then ran meteor and it worked. This indicates Meteor ignoring the dependency in that case, for some reason.

I think this might be considered a bug because all other tools (Webpack, Rollup, Parcel, etc) all successfully build an app regardless of which field dependencies are in, and this is in tuitive: tools (and node module resolution algorithm) care about what is in node_modules, not what is listed in dev vs non-dev dependencies.

Another way to look at this is that having runtime dependencies in dependencies is namely a thing for libraries that run in Node and need to use Node's lookup algorithm at runtime. In this sense, Meteor is treating an app like a library.

If we were to write a plain Node.js app, with devDependencies, then run npm install and finally node server.js (or similar to start the server), it would work. On this note, it will be intuitive for Meteor to also work the same.

The only time devDependencies are ignored in the Node ecosystem is when npm installing a library, and in that case devDependencies are left behind and do not make it to node_modules.

trusktr avatar Feb 09 '22 18:02 trusktr

@denihs incredible I would never had imagined this to be the issue! Thanks a lot! It's consistent with my own experience with Material UI, as it was installed as a dev dependency in Vulcan. We use a Meteor app as a kind of a monorepo just for dev, so we have a lot of dev dependencies.

eric-burel avatar Feb 09 '22 21:02 eric-burel

This needs to be marked as a bug.

trusktr avatar Mar 16 '22 02:03 trusktr

Hi @trusktr,

This was fixed on this PR.

We have already released a new version with this code.

Could you test it?

Just update your app with:

meteor update --release 2.7-rc.4

denihs avatar Mar 18 '22 14:03 denihs

After performing meteor update to 2.7.1, the above reproduction still fails with an error:

❯ meteor --exclude-archs "web.browser.legacy, web.cordova"
[[[[[ ~/deleteme/mapapp ]]]]]                 

=> Started proxy.                             
=> Started HMR server.                        
=> Started MongoDB.                           
W20220407-20:53:38.794(-7)? (STDERR) /home/trusktr/.meteor/packages/meteor-tool/.2.7.1.tb7vzh.a2uk++os.linux.x86_64+web.browser+web.browser.legacy+web.cordova/mt-os.linux.x86_64/dev_bundle/server-lib/node_modules/fibers/future.js:280
W20220407-20:53:38.799(-7)? (STDERR) 						throw(ex);
W20220407-20:53:38.799(-7)? (STDERR) 						^
W20220407-20:53:38.799(-7)? (STDERR) 
W20220407-20:53:38.799(-7)? (STDERR) Error: Cannot find module "/node_modules/dayjs/esm/index.js". Try installing the npm package or make sure it is not a devDependency.
W20220407-20:53:38.799(-7)? (STDERR)     at Module.useNode (packages/modules-runtime.js:647:11)
W20220407-20:53:38.799(-7)? (STDERR)     at module (packages/modules.js:215:8)
W20220407-20:53:38.799(-7)? (STDERR)     at fileEvaluate (packages/modules-runtime.js:336:7)
W20220407-20:53:38.799(-7)? (STDERR)     at Module.require (packages/modules-runtime.js:238:14)
W20220407-20:53:38.799(-7)? (STDERR)     at Module.moduleLink [as link] (/home/trusktr/.meteor/packages/modules/.0.18.0.1i29cl.iter++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/@meteorjs/reify/lib/runtime/index.js:52:22)
W20220407-20:53:38.800(-7)? (STDERR)     at module (imports/day.js:1:1)
W20220407-20:53:38.800(-7)? (STDERR)     at fileEvaluate (packages/modules-runtime.js:336:7)
W20220407-20:53:38.800(-7)? (STDERR)     at Module.require (packages/modules-runtime.js:238:14)
W20220407-20:53:38.800(-7)? (STDERR)     at Module.moduleLink [as link] (/home/trusktr/.meteor/packages/modules/.0.18.0.1i29cl.iter++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/@meteorjs/reify/lib/runtime/index.js:52:22)
W20220407-20:53:38.800(-7)? (STDERR)     at module (server/imports/fixtures.js:1:1)
W20220407-20:53:38.800(-7)? (STDERR)     at fileEvaluate (packages/modules-runtime.js:336:7)
W20220407-20:53:38.800(-7)? (STDERR)     at Module.require (packages/modules-runtime.js:238:14)
W20220407-20:53:38.800(-7)? (STDERR)     at Module.moduleLink [as link] (/home/trusktr/.meteor/packages/modules/.0.18.0.1i29cl.iter++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/@meteorjs/reify/lib/runtime/index.js:52:22)
W20220407-20:53:38.800(-7)? (STDERR)     at module (server/main.js:1:1)
W20220407-20:53:38.800(-7)? (STDERR)     at fileEvaluate (packages/modules-runtime.js:336:7)
W20220407-20:53:38.800(-7)? (STDERR)     at Module.require (packages/modules-runtime.js:238:14)
=> Exited with code: 1
=> Your application is crashing. Waiting for file change.
^C

In particular, note that the missing file does exist:

❯ ls -l node_modules/dayjs/esm/index.js
-rw-rw-r-- 1 trusktr trusktr 12274 Apr  7 20:52 node_modules/dayjs/esm/index.js

trusktr avatar Apr 08 '22 03:04 trusktr

Here is a new reproduction that includes the meteor update:

git clone [email protected]:trusktr/mapapp.git
cd mapapp
git checkout meteor-issue-11900-2
npm install
meteor --exclude-archs "web.browser.legacy, web.cordova"

trusktr avatar Apr 08 '22 03:04 trusktr

Hi, the solution on the PR was to throw a clear error message:

Try installing the npm package or make sure it is not a devDependency..

Did you make sure the package is not a devDependency?

denihs avatar Apr 08 '22 14:04 denihs

Hi,

I find it unexpected that Meteor build differentiates between prod and dev dependencies (no other build engine I know does that), but at least the error message makes it more explicit indeed!

Looks like it is also the case in above repro example: https://github.com/LUMECraft/mapapp/blob/meteor-issue-11900-2/package.json#L30

{
	"devDependencies": {
		"dayjs": "^1.10.7"
	}
}

bobo96run avatar Mar 20 '24 06:03 bobo96run