Port source from CoffeeScript over to Civet#47
Port source from CoffeeScript over to Civet#47Mokshit06 wants to merge 26 commits intoedemaine:mainfrom
Conversation
edemaine
left a comment
edemaine
left a comment
There was a problem hiding this comment.
I've reviewed about half of the files so far. It's looking great! Here are some comments. I'll continue reviewing the other civet files (filter, oripa, viewer) that I haven't looked at yet.
I also turned on strict type checking, which has revealed a bunch of errors, but we can probably deal with many of them later...
I'm getting a problem with npm run build though:
> fold@0.13.0 build
> vite build
vite v4.5.0 building for production...
watching for file changes...
build started...
✓ 2 modules transformed.
Unexpected token (Note that you need plugins to import files that are not JavaScript)
file: C:\Users\edemaine\Projects\fold\src\convert.civet.tsx:1:10
1: var modulo: (a: number, b: number) => number = (a, b) => (a % b + b) % b;
^
2: import * as geom from "./geom.civet"
3: import * as filter from "./filter.civet"
Any idea why the type annotation didn't get removed there?
| @@ -0,0 +1,24 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
I think I'd rather name this file foldviewer.html (as before). Maybe we eventually want other HTML files (e.g. docs) deployed too?
| examples: { | ||
| Default: 'examples/simple.fold', | ||
| 'Flexicube Unit': 'examples/box.fold', | ||
| 'Square Twist': 'examples/squaretwist.fold', |
There was a problem hiding this comment.
Should we add cube-cp/cube-folded? (Makes me think it'd be cool to add to Fold Viewer an option like "unfold" which calls your new routine; and when edges_foldAngle is availabe, a "fold" button.)
| "version": "0.13.0", | ||
| "description": "FOLD file format for origami models, crease patterns, etc.", | ||
| "main": "lib/index.js", | ||
| "main": "dist/index.civet.js", |
There was a problem hiding this comment.
I wonder whether we should keep .civet in all the extensions. I think it'd be better to remove them. Is this easy to do, or does it break all the imports?
There was a problem hiding this comment.
With the way the current unplugin works, it would break all the imports. Though we could maybe the file matcher for transpiled civet in it from "[path].civet.js" to "civet:[path].js", in which case the final extension should just be js. I'm not sure if it would make much of a difference though
| "./oripa": { | ||
| "import": "./dist/oripa.civet.js", | ||
| "require": "./dist/oripa.civet.mjs" | ||
| } |
There was a problem hiding this comment.
We aren't generating dts files here yet, but once we should just be able to add types.civet as another entrypoint in vite config
src/convert.civet
Outdated
| edge | ||
| fold | ||
|
|
||
| export function unfoldedGeometry(fold: Fold, rootFace = 0) |
There was a problem hiding this comment.
There is a ton of shared code between these three functions. Can we factor them out into a generic function that gets called three different ways?
Also, it seems flatFoldedGeometry and flatUnfoldedGeometry are identical, so we don't need the latter (or could alias latter to former).
Did you enable RE strict type checking, I think we should be adding runtime assertions rather than non null-assertions for fold property access |
2 participants
Related to #45.
Moves coffeescript source over to Civet and adds type definitions. Haven't removed the CS source code yet, as
@danielx/civetdoesn't have the unplugin plugin exported yet in the latest released npm version, so we can't publish it.We can remove that once the type definitions and type names have been finalised and the build system can support the compilation from civet -> js + dts