[Bug]: Implementation does not support code minification with Class/Function renaming

[quasarchimaere]

What happened?

I posted this on the people.thatopen.com message board already, but think this is also worthy of a sep. ticket here:

Usually js webapp use a bundler or some code minification tool in order to reduce the size of the apps, these usually include treeshaking (removal of unused files/code etc.) as well as the minification of the code itself. The minification includes renaming of vars/classes/functions and this is where the problem arises:

Currently the web-ifc implementation does not support renaming of classes in the build process, why:

 ifcApi.WriteRawLineData(modelID, {
    ID: nextExpressID,
    type: IFCPROPERTYSINGLEVALUE,
    arguments: [
      { value: "Test", type: 1 },
      null,
      { value: "Value", valueType: 1, type: 2, label: "IFCLABEL" },
      null,
    ],
  });

Seeing this code snippet above (especially the idx 2 of the arguments) you see that when trying to write a Line to the model with web-ifc there is some form of transformation going on, this transformation is made within a single function of the code named labelise which looks like this:

function Labelise(tapeItem) {
  tapeItem.value = tapeItem.value.toString;
  tapeItem.valueType = tapeItem.type;
  tapeItem.type = 2;
  tapeItem.label = tapeItem.constructor.name.toUpperCase()

  return tapeItem;
}

There are two sep. Issues in this Function:

1. The Function is changing the original Object and thus would make subsequent calls faulty or at least superflous (since it changes the original parameter, even though i assume that the change is only needed for the ToRawLineData param list)
2. And this is the one i try to address in this Issue: the label param of the tapeItem is calculated upon the constructor name. e.g. an IfcLabel object as tapeItem will be translated to a label with the string "IFCLABEL"... so far so good

HOWEVER:
as minification is in progress and in the production environment everything changes, IfcLabel as class might turn into a Class Named "Xzy" and thus the Labelise function would not create the proper Arguments for writing a Line to the model any longer

Suggestion Fix for Issue 1 (change of input tapeItem when Labelising):
Change the Labelise function to return a new object, and add a safeguard if the Labelise function is called with something that already has been labelised (we currently apply a patch that does this, but i assume that the safeguard might not be necessary for the general codebase)

function Labelise(tapeItem) {
  if(tapeItem.label) {
    return {...tapeItem};
  }
  return {
      value: tapeItem.value.toString(),
      valueType: tapeItem.type,
      type: 2,
      label: tapeItem.constructor.name.toUpperCase(),
  };
}

Suggestion Fix for Issue 2 (use of constructor name)
Add a static string to each Class and replace the tapeItem.constructor.name.toUpperCase() with the appropriate string

e.g:
add static string LabelString = "IFCLABEL"; to class IfcLabel
instead of tapeItem.constructor.name.toUpperCase() use tapeItem.LabelString

Version

0.0.43+

What browsers are you seeing the problem on?

No response

Relevant log output

No response

Anything else?

No response

[beachtom]

I will take a look of this - but for clarity - does it mean that some bundling breaks the code? Or is it that it could be minified more than it currently is?

[quasarchimaere]

excerpt of our vite config (we use svelte/sveltekit and vite which uses rollup and esbuild under the hood)

ssr: {
    noExternal: [/*others*/ "openbim-components", "bim-fragment", "openbim-clay",,"three"],
  },
  optimizeDeps: {
    exclude: [/*others*/,"three"],
  },
[...]
esbuild: {
    // Minifying the Identifiers is not possible otherwise the web-ifc BL does not work anymore since they use classnames as labelers (web-ifc 0.0.43)
    minifyIdentifiers: false,
  },
build: {
 minify: true
}

explanation:
ssr.noExternal: had to include openbim-components etc because otherwise the build/dev mode doesnt work (this is not unusual for some dependencies and i can live with that)
three js had to be included in both ssr.noExternal and optimizeDeps.exclude because otherwise obc would not find the three/jsm/example imports (this can be linked to the ssr.noExternal though, i am not sure, but its also something i can live with for now, and maybe if i have time i can dig deeper into what this all means)

esbuil.minifyIdentifiers: false --> if this is omitted and minify is true, the build runs, succeeds and also deploys, and you will never run into problems until you try to save a model to a file again or (and this is how we found out) try to utilize the ToRawLine Functions (Labelise) to create arrays of line changes (incl. writing new lines), otherwise this error is completely dorment, and if anything only visible in the build or build preview (thx to the vite preview option) in an hmr(hot module reload) dev setup this will not show up so you wont see this during development

tl;dr:

you have to actively change the default settings in vite/rollup to make the build work after minification

[beachtom]

OK I think this is now fixed - can you let me know and I am happy to look again if still not minifying

[quasarchimaere]

i am not sure if your commit would work to be honest

src/schema-generator/gen_functional_types.ts looking at line 66 it seems like the added parameter that will be used as the label is passed as parameter, and is not uppercased it also adds more complexity to the function itself, i am not sure if it isnt prone to be buggy and is harder to be maintained.

wouldnt it be better to use a specific static member in the generated sources that defines this?

[quasarchimaere]

i am not sure how to try it out without using a released version of web-ifc in my project, do you have an guide/howto for building and testing this project from sources so i could test it? if so i would gladly try it for you

[beachtom]

It should definetly be uppercased - that was my mistake. I have fixed it.

The reason i don't want to add a static property is it will add them to all classes - not just the ones that use Labelise.

To test it (once all the pipelines have run) there is a download linked from the readme on the repository. If you extract this and place it in node_modules/web-ifc - it should then use the new version .

Let me know how it goes

[quasarchimaere]

thx for the short how-to, will try this out tomorrow, and let you know

[beachtom]

I actually managed to fix a few things - including not needing the name parameter - so I hope it works better now

[quasarchimaere]

i just checked it, and you fixed it

thx for adding the name to the generated class for all classes, this is useful to us also

@beachtom do you have an eta when this is going to be deployed as a new web-ifc version on npm?

[beachtom]

Probably in the next week or so.