donmccurdy / glTF-Transform

glTF 2.0 SDK for JavaScript and TypeScript, on Web and Node.js.
https://gltf-transform.dev
MIT License
1.41k stars 149 forks source link

Bug on prune on MacOS & nodejs #1140

Closed Makio64 closed 1 year ago

Makio64 commented 1 year ago

Describe the bug The following code is working well on Nodejs Windows but got errors on Nodejs MacOS. It seems related to nodeTreeShake / treeShake function getting undefined when call in themself.

To Reproduce Steps to reproduce the behavior:

this is all the commands i tested which fail( and worked as expected under nodejs windows )

// BUG1 Failed to get file info: ReferenceError: nodeTreeShake is not defined at prune
await document.transform(flatten())
await document.transform(weld({ tolerance: params.weldTolerance, toleranceNormal: params.weldToleranceNormal })) 
await document.transform(simplify({ simplifier: MeshoptSimplifier, ratio: params.simplifyRatio, error: params.simplifyError }))

//BUG2 Failed to get file info: ReferenceError: treeShake is not defined at prune
await document.transform(reorder({ encoder: MeshoptEncoder }), quantize({ pattern: /^(POSITION|TEXCOORD|JOINTS|WEIGHTS)(_\d+)?$/ })) 

Expected behavior No error on both.

Versions:

Additional context Compile under electron-vite Windows & Mac use latest nodejs version

donmccurdy commented 1 year ago

Hmmm that's a surprise! Do you mind checking if you get the same error in Node.js v18? I develop on macOS, so I'm not sure what might be different here.

Makio64 commented 1 year ago

Yeah I was also surprise.

I just tested and got the same errors with Nodejs v18.9.0

I'm using electron-vite latest, electron latest and this in my .jsconfig for the project :

{
  "compilerOptions": {
    "module": "esnext",
    "target": "esnext",
    "baseUrl": "./src/renderer",
      "paths": {
        "@/*": ["./src/*"]
      }
  },
  "include": ["src"]
}

also excepted prune() all the other commands seems to work fine

Makio64 commented 1 year ago

I rectify, I just switch back on windows this morning and same bug. I revert to older gltf-transform version 3.7.0 fine 3.7.1 fine 3.7.2 fine 3.7.3 fine ( both mac / windows ) 3.7.4 broken ( same bug on both mac / windows )

javagl commented 1 year ago

Same here (Windows 10, Node v18.12.1)

Before I saw this issue, I zoomed into that by creating an empty project with a package.json of

{
  "name": "gltf-transform-version-issue",
  "version": "0.0.0",
  "dependencies": {
    "@gltf-transform/core": "3.7.4",
    "@gltf-transform/functions": "3.7.4"
  },
  "devDependencies": {
    "ts-node": "^10.9.1",
    "typescript": "^4.8.3"
  }
}

and a VersionIssue.ts like

import { Document } from "@gltf-transform/core";
import { prune } from "@gltf-transform/functions";

async function run() {

  const document = new Document();
  const node = document.createNode();
  const scene = document.createScene();
  scene.addChild(node);
  await document.transform(prune());
}

run();

Doing npm install npx ts-node ./VersionIssue.ts works fine when the version is pinned to 3.7.3, but with 3.7.4 it prints

C:\glTF-Transform-Version-Issue\node_modules\@gltf-transform\functions\src\prune.ts:107
                if (propertyTypes.has(PropertyType.NODE) && !options.keepLeaves) root.listScenes().forEach(nodeTreeShake);
                                                                                             ^
ReferenceError: nodeTreeShake is not defined
    at transform (C:\glTF-Transform-Version-Issue\node_modules\@gltf-transform\functions\src\prune.ts:107:94)
    at body (C:\glTF-Transform-Version-Issue\node_modules\@gltf-transform\core\src\document.ts:214:10)
    at t (C:\glTF-Transform-Version-Issue\node_modules\@gltf-transform\core\src\document.ts:211:15)
    at _forOf (C:\glTF-Transform-Version-Issue\node_modules\@gltf-transform\core\src\document.ts:231:5)
    at vt.transform (C:\glTF-Transform-Version-Issue\node_modules\@gltf-transform\core\src\document.ts:212:49)
    at run (C:\glTF-Transform-Version-Issue\VersionIssue.ts:10:18)
    at Object.<anonymous> (C:\glTF-Transform-Version-Issue\VersionIssue.ts:13:1)
    at Module._compile (node:internal/modules/cjs/loader:1159:14)
    at Module.m._compile (C:\glTF-Transform-Version-Issue\node_modules\ts-node\src\index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
donmccurdy commented 1 year ago

I see, thanks @javagl and @Makio64! I'm still baffled by what's broken in 3.7.4, but I'll revert the changes for now and publish 3.7.5 soon, so at least the latest version will be stable.

donmccurdy commented 1 year ago

I've published v3.7.5 (duplicate of v3.7.3) for now. I will need to find some time to look into what's wrong on v3.7.4.

donmccurdy commented 1 year ago

Changes in #1127 hit a bug in the build system, breaking the prune() function only in the CommonJS builds.

Context:

Still trying to figure out if I can configure Microbundle to output async/await to CommonJS builds without transpiling, or what alternatives are available if not.

donmccurdy commented 1 year ago

I've refactored the prune() function to avoid the transpiler bug, and published a new release to v3.8. In v4 I hope to make some larger build changes that would skip this transpiler step.