npm update for 3.2.0 fails on Windows 10: can't find postinstall.js

anders8 commented 1 year ago

Description

I've been using model-viewer for a year with no trouble in an Angular app, this shows up only with the new 3.2.0 release. I see that there's a new script... but it's not being found. Take a look at where it's trying to find it though: is it about two directories too high?

Perhaps there's a difference between OSs where the previous command is leaving the working directory?

Perhaps there's a slight OS difference in the working directory for the command vs the execution? "postinstall": "if [ -f '../../scripts/postinstall.js' ]; then node ../../scripts/postinstall.js; fi",

I assume that the intent here is to be finding D:_mystuff_\node_modules@google\model-viewer\scripts\postinstall.js

But if I do a npm pack --dry-run @google/model-viewer there isn't a scripts directory, or the postinstall.js script coming down anywhere. And to that, one could reasonably say "well, it's only supposed to try to run it if it finds it" and yeah, I don't have an answer for you there.

Additional notes:

same result with and without the "three" package installed
same result in cmd and powershell, regular user and admin
same result for npm update and npm install if I scrap node_modules and package_lock

Version

model-viewer: v3.2.0

Browser Affected

None, npm install issue

OS

[x] Windows 10

Env

npm: 9.6.7 node: 18.12.1

unrealsmart commented 1 year ago

i use pnpm, but i have the same problem

Progress: resolved 1422, reused 1385, downloaded 0, added 0, done
node_modules/.pnpm/@google+model-viewer@3.2.0/node_modules/@google/model-viewer: Running postinstall script, failed in 80ms
.../node_modules/@google/model-viewer postinstall$ node ../../scripts/postinstall.js
│ node:internal/modules/cjs/loader:998
│   throw err;
│   ^
│ Error: Cannot find module 'D:\xxx\node_modules\.pnpm\@google+model-viewer@3.2.0\node_modules\scripts\postinstall.js'
│     at Function.Module._resolveFilename (node:internal/modules/cjs/loader:995:15)
│     at Function.Module._load (node:internal/modules/cjs/loader:841:27)
│     at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
│     at node:internal/main/run_main_module:22:47 {
│   code: 'MODULE_NOT_FOUND',
│   requireStack: []
│ }
└─ Failed in 81ms at D:\xxx\node_modules\.pnpm\@google+model-viewer@3.2.0\node_modules\@google\model-viewer
 ELIFECYCLE  Command failed with exit code 1.

When I try to run pnpm install @google/model-viewer@3.1.1 everything works fine!

vogloblinsky commented 1 year ago

Same thing on macOS 13.4.1, Node.js 18.15.0, npm 9.5.0

anders8 commented 1 year ago

@vogloblinsky ooo, thanks for the reminder, I've updated my post with the node and npm version.

@unrealsmart Yours is pretty interesting, because it's at least looking in the directory I thought it would be looking in. Note that yours is at least looking in the model-viewer directory, while mine is not! Is there an environmental variable being relied upon here?

But doesn't get to the "the script isn't in the pack" issue.

Frank3K commented 1 year ago

Looking at commit 0f55b439b657b7a0be6b4fa5952268e3dbc27fe5, I don't think the postinstall should be part of the distributed npm package.

anders8 commented 1 year ago

@Frank3K Not sure from your phrasing which you're saying:

"I looked at #0f55b43 and it shows postinstall.js as part of the distributed npm package, and my opinion is that inclusion is unnecessary."
"I looked at #0f55b43 and it doesn't seem like postinstall.js gets included in the distribution."

Frank3K commented 1 year ago

@Frank3K Not sure from your phrasing which you're saying:

* "I looked at #0f55b43 and it shows postinstall.js as part of the distributed npm package, and my opinion is that inclusion is unnecessary."

* "I looked at #0f55b43 and it doesn't seem like postinstall.js gets included in the distribution."

Option c: something else.

A reference to postinstall.js is included in package.json but the postinstall script is not included in the npm package.

Since the postinstall script is for updating parts of the website (0f55b43) I think it should not be included in the npm package (which is the case atm), but also should not be referenced in the package.json (which it is atm).

I now see the issue has already been fixed in https://github.com/google/model-viewer/pull/4363 by @btopro, but that was merged after version 3.2.0 was released.

I think a new npm package version solves the issue. Cc: @elalish.

anders8 commented 1 year ago

Nice... sounds like time to sit tight. Thanks for the clarifying explanation and the sleuthing. 👍 And thanks @btopro for the fix!

anders8 commented 1 year ago

Can verify, 3.2.1 has fixed the issue. 👍

google / model-viewer