setExtension fails when trying to call _validateParent

[marwie]

Describe the bug
When calling setExtension on e.g. a texture during read I'm getting an error caused by _validateParent not being defined

md TypeError: e.t is not a function at wt.setExtension (file:///C:/git/needle-gltf-extensions/modules/glTF-Transform/packages/core/dist/core.modern.js:1:13709) at C:\git\needle-gltf-extensions\package\dist\extensions\NEEDLE_compression_texture\NEEDLE_compression_texture.js:26:21 at Array.forEach (<anonymous>)

To Reproduce
Steps to reproduce the behavior:

1. create custom extension
2. during read get textures and try to add a extension to one of the textures

Expected behavior
no exception, extension is added

Versions:

• Version: 3.1.1
• Environment: node 18

Additional context

export const EXTENSION_NAME = "NEEDLE_compression_texture";

export declare interface NEEDLE_compression_texture_schema {
    mode: string;
    slot: string;
    /** quality in range of 0 - 1 */
    quality: number;
    maxSize: number;
}

const debugLog = false;

export class NEEDLE_compression_texture extends Extension {

    static EXTENSION_NAME = EXTENSION_NAME;
    extensionName: string = EXTENSION_NAME;

    create(toktxInfo: NEEDLE_compression_texture_schema) {
        const ext = new NEEDLE_compression_texture_property(this.document.getGraph(), "");
        ext.model = toktxInfo;
        return ext;
    }

    read(context: ReaderContext): this {
        if (!context.jsonDoc.json.textures) return;
        if (debugLog) console.log("READ " + EXTENSION_NAME);
        context.textures?.forEach((tex, _) => {
            const textureDefinition = getTextureDefinition(context, tex)
            if (textureDefinition?.extensions && textureDefinition.extensions[EXTENSION_NAME]) {
                const prop = new NEEDLE_compression_texture_property(this.document.getGraph(), EXTENSION_NAME);
                prop.model = textureDefinition.extensions[EXTENSION_NAME] as NEEDLE_compression_texture_schema;
                tex.setExtension(EXTENSION_NAME, prop);
            }
        });
        return this;
    }

    write(context: WriterContext): this {
        if (debugLog) console.log("WRITE " + EXTENSION_NAME);
        const root = this.document.getRoot();
        const textures = root.listTextures();
        for (const tex of textures) {
            const ext = tex.getExtension(EXTENSION_NAME) as NEEDLE_compression_texture_property;
            if (ext) {
                const def = getTextureDefinition(context, tex);
                if (def) {
                    def.extensions = def.extensions || {};
                    def.extensions[EXTENSION_NAME] = ext.model;
                }
            }
        }
        return this;
    }
}

export class NEEDLE_compression_texture_property extends ExtensionProperty {
    static EXTENSION_NAME = EXTENSION_NAME;
    readonly extensionName: string = EXTENSION_NAME;
    parentTypes: PropertyType[] = [PropertyType.TEXTURE];
    propertyType = EXTENSION_NAME;
    init(): void { }
    model: NEEDLE_compression_texture_schema;
}

[donmccurdy]

Let's move this to a discussion — we have a lot of extensions implemented without this error, so until/unless this is reproducible as a clear bug, I'm guessing it will be more of a discussion around extension implementation.

[marwie]

Yes I would guess so :) It worked fine in 2.x tho but tried upgrading yesterday

[donmccurdy]

Can you provide further details so that I can run this code? I'm not sure how to test the issue without a file to read. One red flag I do see, the extension property's init() function cannot be empty. extensionName, propertyType, and parentTypes should be declared there. Because of how inheritance works in JavaScript, that's the only way I've found to be sure they'll be available when the constructor of a parent class runs.

glTF-Transform/packages/extensions/src/khr-materials-clearcoat/clearcoat.ts

Lines 39 to 43 in 32dc66e

	protected init(): void {
	this.extensionName = KHR_MATERIALS_CLEARCOAT;
	this.propertyType = 'Clearcoat';
	this.parentTypes = [PropertyType.MATERIAL];
	}

[marwie]

Currently installed is 4.9.4

I'm trying to fix the init function and see if I'm able to find a repro

[marwie]

here is a glb with that extension Cube.zip

I changed init and made sure to rebuild the dist and re-install gltf-transform but it didnt help

Could it be caused by the tsconfig set to CommonJS ?

[marwie]

That seems to be it. Changed my tsconfig to ESNext and NodeNext as in your package and updated all the imports

{
    "compilerOptions": {
        "target": "ESNext",
        "outDir": "dist",
        "declaration": true,
        "moduleResolution": "NodeNext",
        "module": "ESNext",
        "esModuleInterop": true,
        "skipLibCheck": true
        
    },
    "include": [
        "src/index.ts"
    ]
}

[donmccurdy]

😱 ... well that's incredibly confusing. Could I ask what was the compile target before ESNext? I can kind of imagine that targeting ES5 (no classes) might break things but I am not sure why output to CommonJS or ES6+ would be a problem.

Methods and properties prefixed by _ in the library are minified during compilation, to reduce library size a bit. _validateParent is one example. But these methods are only called internally, so i'm not sure how the TypeScript configuration could lead to this error.

[marwie]

Previously that was set to Node and CommonJs

{
    "compilerOptions": {
        "target": "ESNext",
        "outDir": "dist",
        "declaration": true,
        "moduleResolution": "Node",
        "module": "CommonJS",
        "esModuleInterop": true,
        "skipLibCheck": true
        
    },
    "include": [
        "src/index.ts"
    ]
}

[Samsy]

Hello !

Got the same problem as well !
With a basic setup unlit

const unlitExtension = doc.createExtension(KHRMaterialsUnlit);
   // Create an Unlit property.
   const unlit = unlitExtension.createUnlit();

   const material = doc.createMaterial('myMaterial')
       .setBaseColorFactor([1, 1, 1, 1]) // RGBA

   // Attach the property to a Material.
   material.setExtension('KHR_materials_unlit', unlit);
   

t is not a function
at ct.setExtension (core.modern.js:1:13682)

[marwie]

Hi, I assume this didnt fix it for you? #857 (reply in thread)

[donmccurdy]

Based on solutions in other threads, this is "dual package hazard" (see https://nodejs.org/api/packages.html). The solution will depend on how you're compiling your code, but targeting ESM output (not CJS) is probably best.

[Samsy]

So, we figured it out, and seems to be our mistake.

We copied and pasted nodejs scripts into web scripts, using both import and requires, which mix the sources between cjs and moderns which results in those errors !