jscad / OpenJSCAD.org

JSCAD is an open source set of modular, browser and command line tools for creating parametric 2D and 3D designs with JavaScript code. It provides a quick, precise and reproducible method for generating 3D models, and is especially useful for 3D printing applications.
https://openjscad.xyz/
MIT License
2.58k stars 505 forks source link

feat(modeling): V2 api compatibility shim #1323

Open platypii opened 4 months ago

platypii commented 4 months ago

This PR adds a "V2 compatibility shim" to V3.

In PR #1294 we flattened the JSCAD package heirarchy so that functions like union and extrudeLinear are now in the top level package object:

-import { booleans } from '@jscad/modeling'
-const { union } = boolean
+import { union } from '@jscad/modeling'

But what about all the hundreds of designs out there for V2? They will all break, annoyingly, in the migration to V3.

This PR keeps the flattened structure for V3 but ALSO exports a similar structure to V2. This is not 100% api compatible, due to some fundamental changes in V3, but it is pretty close, and I tried a couple of my designs from V2 against this, and they actually all just-worked!

Feedback welcome, but I think this would make the transition from V2 to V3 much smoother for users of JSCAD.

All Submissions:

z3dev commented 4 months ago

I kind of like it and kind of don't, but as @hrgdavor said... these changes help in the transition from V2 to V3.

But is it enough? Would a full V2 compatible shim be better?

platypii commented 4 months ago

Would a full V2 compatible shim be better?

This is mostly a full compatibility shim, I tried to make it as complete as I reasonably could. Here is a list of some of the known differences between V2 and this V3-shim:

There are probably some other discrepancies that I missed. But overall, I think these are minor. I am open to suggestions on where to make this shim more api-compatible, but I think this is a pretty good first attempt.

z3dev commented 4 months ago

I get it but a little skeptical.

To use the shim requires changes to the imports. So, why not just make a V2 fully compatible shim, which will also requires changes to imports?

I'm not sure which is better.

V2 was criticized by those that like the 'object' chaining. But it provided a new core set of functionality, which surpassed V1 in many ways. Hopefully, V3 will also do the same.

I think the shim is fine, but really think it should be V2 as much as possible. Is it possible?

platypii commented 4 months ago

To use the shim requires changes to the imports.

What do you mean? By adding this shim, users do NOT need to change imports from V2 designs. With the shim, this will still work:

const jscad = require('@jscad/modeling')
const { cube } = jscad.primitives

const main = () => cube({ size: 8 })

module.exports = { main }

But so will the more modern V3 syntax:

import { cube } from '@jscad/modeling'
export const main = () => cube({ size: 8 })

This seems like the best of both worlds. We can deprecate the old syntax, and remove the shim in V4?

z3dev commented 3 months ago

So there are several things being discussed here...

  1. transcompiling CommonJS to ES5
  2. adding the V2 namespaces back to V3
  3. adding full or partial V2 compatibility
  4. support by the CLI

For transcompiling, this is already supporting in the JSCADUI (Nice!).

Adding the V2 namespaces back seems reasonable, as that will ease the transition of designs from V2 to V3.

As for the full V2 compatibility, I think this should be possible but some special functions have to be written and tested. This compatibility shim could be part of this package, but it would have to be imported specifically by designs. And the compatibility shim might be better as a separate package, again possible.

Also, I think @hrgdavor is now looking at some ways to identify V1 vs V2 vs V3 designs. It would be cool if this could be automatic. But maybe a design can select the API version in a comment, e.g. 'V2'.

Finally, the CLI will become another problem. The CLI may need a total re-write to support transcompiling, different versions of the API, etc.

z3dev commented 3 months ago

@platypii can we focus these changes on just point # 2, adding the V2 namespaces back to V3? it's shouldn't be that different.

then we can decide where to put the V2 compatibly shim, and make it 100% compatible, if possible.

hrgdavor commented 3 months ago
  1. transcompiling CommonJS to ES5

It is actually the oposite :) ... it is impossible as far as I looked, to execute ES5 modules the way we need to for jscad worker. ES5 import statements are converted ro require calls so a properly configured require function can be injected. The injected require must have the context what the current path is, so it can resolve relative imports.

We can make a reasonable effort to recognize V1,V2,V3 but we will need to provide a comment with some kind of annotation

some errors could be a giveaway if wrong version is recognized

z3dev commented 3 months ago

It is actually the oposite :) ... it is impossible as far as I looked, to execute ES5 modules the way we need to for jscad worker. ES5 import statements are converted ro require calls so a properly configured require function can be injected. The injected require must have the context what the current path is, so it can resolve relative imports.

Wow! That's really a surprise. So, the dynamic imports really don't help? Big shock!

We can make a reasonable effort to recognize V1,V2,V3 but we will need to provide a comment with some kind of annotation

This is probably the best solution, as this could be the standard going forward.

hrgdavor commented 3 months ago

So, the dynamic imports really don't help? Big shock!

I did not try to inject override for dynamic import. Trying to eval a script that uses the import .... from '...' just fails, so at that point it seemed just better to convert to CJS require. Luckily with transpilers working so well for years, this was not a big deal, and with sourcemaps, you can still see proper error reporting and debugging.

also there was compatibility issue for dynamic import in workers last time I looked (if I remember correctly).

platypii commented 3 months ago

can we focus these changes on just point # 2, adding the V2 namespaces back to V3? it's shouldn't be that different. then we can decide where to put the V2 compatibly shim, and make it 100% compatible, if possible.

I am not clear on what you are asking for here? You want the exports in the v2 and v3 namespaces, but none of the compatibility functions? If that is the intent I have to say... why?? You are asking for a world where someone who had a project that used V2 and now upgrades to V3 will have imports that seem to work, but break in mysterious ways. That seems less compatible, and worse than not having the V2-like exports at all!

If you are worried about bloat from adding the functions here? I would agree, except this PR is only 41 lines of code and provides backward compatibility for most jscad V2 designs to upgrade seamlessly to V3. It is a tiny addition, with great benefits. I'm not sure why we wouldn't do this for users? (See #1326 for example of user pain)

z3dev commented 3 months ago

can we focus these changes on just point # 2, adding the V2 namespaces back to V3? it's shouldn't be that different. then we can decide where to put the V2 compatibly shim, and make it 100% compatible, if possible.

I am not clear on what you are asking for here?

In simple terms, a 100% compatible V2 shim. These changes look good initially but there are differences still. And V3 changes are not complete yet. For example, the use of a shared set of attributes for color, etc.

Also, these V2 compatibility changes are being added directly into the V3 API. V2 and V3 in the same package? Why? Will V1 be added as well? There must be another way to handle this.