Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TypeScript / tsconfig.json set "strict": true + refactor #153

Closed
vatro opened this issue Aug 28, 2022 · 3 comments · Fixed by #185
Closed

TypeScript / tsconfig.json set "strict": true + refactor #153

vatro opened this issue Aug 28, 2022 · 3 comments · Fixed by #185
Assignees
@vatro
Labels
code quality refactor
Milestone
1.0.0-next.1

Comments

@vatro
Copy link
Owner

vatro commented Aug 28, 2022 •
edited
Loading

yeah 🤔... think about it: to do or not to do / why I haven't done in the first place? (I remember this was a decision though)

  • Ok, now I remember, It was because I had to add undefined to every prop union type, it was kind of annoying in the early development phase.
  • Also: Svelte development was not strict per default after Svelte 3 release, this changed recently in create-svelte.

This has to be done before next.1
@vatro vatro added this to the 1.0.0-next.X milestone Aug 28, 2022
@vatro vatro self-assigned this Aug 28, 2022
vatro pushed a commit that referenced this issue Aug 29, 2022
tsconfig.json explicitly set "strict": false
28547c4 
TODO: consider setting `"strict": true`, see #153
vatro pushed a commit that referenced this issue Aug 29, 2022
SvelthreeGLTF.ts improve dx and refactor (add new) types (see #136)
543fe00 
- rename interface `ITreeMember` to `ISvelthreeGLTFTreeMapMember`
- `GLTF` is now passed via constructor, e.g. `const foo: SvelthreeGLTF = new SvelthreeGLTF(loaded_gltf_file: GLTF)`
- `apply(...)` now uses a `Canvas` component reference instead of a `Canvas` DOM Element.
- various `strict: true` related changes (due to `strict: true` in the SvelteKit tests project, see also #153 )
vatro pushed a commit that referenced this issue Aug 29, 2022
LightTarget.ts "strict": true related changes (see #153)
a440f75
@vatro vatro changed the title TypeScript / tsconfig.json consider setting "strict": true + refactor TypeScript / tsconfig.json set "strict": true + refactor Nov 2, 2022
@vatro vatro modified the milestones: 1.0.0-next.X, 1.0.0-next.1 Nov 2, 2022
vatro pushed a commit that referenced this issue Nov 3, 2022
tsconfig.json set strict to true (#153)
d27e2d2
vatro pushed a commit that referenced this issue Nov 17, 2022
Changes before new svelthree-stores referencing approach (untested) (…
195ba6b 
…#153)
vatro pushed a commit that referenced this issue Nov 17, 2022
Add new store reference to all generated components (#153)
a642697
vatro pushed a commit that referenced this issue Nov 17, 2022
Replace $svelthreeStores[sti] with new store reference (#153)
b0007f5
vatro pushed a commit that referenced this issue Nov 17, 2022
Fix OrbitControls component's strict errors (#153)
65cfcf9
vatro pushed a commit that referenced this issue Nov 17, 2022
Change svelthreeStores's type to include null values (#153)
780d479
vatro pushed a commit that referenced this issue Nov 17, 2022
Changes related to new svelthreeStores type (includes null array …
8daa65a 
…items) (#153)

`store` can be `null`.
vatro pushed a commit that referenced this issue Nov 17, 2022
Better sProps creation on instance change (#153)
caf4e42
@vatro vatro pinned this issue Nov 17, 2022
vatro pushed a commit that referenced this issue Nov 17, 2022
Remove obsolete eslint-disable-next-line no-undef (#153)
a868aef
@vatro
Copy link
Owner Author

vatro commented Nov 17, 2022

Current Status

svelte-check found 0 errors, 0 warnings, and 0 hints
no eslint errors / warnings

Atm still untested!

vatro pushed a commit that referenced this issue Nov 17, 2022
Make new store / stores reference available on initialization (#153)
b5a610c 
Make new `store` reference available on initialization, not in a reactive statement / `onMount`.
vatro pushed a commit that referenced this issue Nov 17, 2022
Log debug not error if sProps were not recreated (#153)
69e5140
vatro pushed a commit that referenced this issue Nov 17, 2022
Fix index_in_cameras conditional in Camera components' `handle_in…
914b65c 
…stance_change()` (#153)
@vatro
Copy link
Owner Author

vatro commented Nov 18, 2022

Current Status

After first tests and fixes (see above) -> looking quite good! 🤞
Continuing fixing in this thread ...

vatro pushed a commit that referenced this issue Nov 21, 2022
Change sProps recreation conditional in handle_instance_change() (#…
20a6945 
…153)

No `props` provided -> silent, but catch possible errors when creating new `SvelthreeProps` instance.
vatro pushed a commit that referenced this issue Nov 21, 2022
Fix *Light components' add_helper logic (#153)
9c7d660 
Don't create helper if there's already one (don't double create). This happened due to changed reactive conditional (removed `!light.userData.helper`)
vatro pushed a commit that referenced this issue Nov 21, 2022
Don't log error in *Light components when unable to remove non-ex…
d43a6a8 
…istent helper (#153)

instead log `debug` (atm) as an alternative to being silent.
vatro pushed a commit that referenced this issue Nov 21, 2022
Fix *Camera components' add_helper logic (#153)
3206309 
Analog `*Light` components.
vatro pushed a commit that referenced this issue Nov 21, 2022
Don't log error in *Camera components when unable to remove non-e…
e7cfa7b 
…xistent helper (#153)

Analog `*Light` components
vatro pushed a commit that referenced this issue Nov 21, 2022
Change reactive raycasting enable/disable logic (block) (#153)
3f7da30 
Put `scene` in top reactive statement condition, since `scene` instance is essential / prevent obsolete `error`logging on initialization.
vatro pushed a commit that referenced this issue Nov 21, 2022
Fix reactive logic bound to store ($svelthreeStores[sti]) changes (#…
7a2a331 
…153)

Use `$` prefixed store reference (not `store`) in these cases:
- in reactive conditions
- when setting a store value: casted to `StoreBody` -> we know the store value is truthy / not `null` because the `store` reference is truthy. REMARK: this would unfortunately be marked as `error` in `strict` mode (VS Code) / `$svelthreeStores[sti]` not recognized as truthy inside the if-statement:
```
if ($svelthreeStores[sti]) {
  $svelthreeStores[sti].foo = value
}
```
so instead we do this:
```
const store = $svelthreeStores[sti]
if (store) {
  ;($svelthreeStores[sti] as StoreBody).foo = value
}
```
vatro pushed a commit that referenced this issue Nov 21, 2022
Fix update_light_target() in *Light components with target (#153)
d186d52
vatro pushed a commit that referenced this issue Nov 21, 2022
Fix OrbitControls component (#153)
f64958d 
Wasn't functioning correctly after recent changes.
vatro pushed a commit that referenced this issue Nov 22, 2022
OrbotsControls use brackets with reactive conditional statement (#153)
c96cb83
vatro pushed a commit that referenced this issue Nov 23, 2022
OrbitControls Fix error logging condition in `create_orbitcontrol…
a9ee816 
…s()` (#153)
vatro pushed a commit that referenced this issue Nov 23, 2022
Remove .js from three/examples/jsm/ import paths (#153)
4b5a18c 
not needed (anymore?)
vatro pushed a commit that referenced this issue Nov 23, 2022
SvelthreeInteraction Refactor / fix logging in set_block_status() (…
8f47a0d 
…#153)
vatro pushed a commit that referenced this issue Nov 23, 2022
LoadedGLTF Log warning instead of an error if url was not provide…
19fae0a 
…d at initialization (#153)
vatro pushed a commit that referenced this issue Nov 23, 2022
Type SvelthreeAnimationFunction, 'foo' not optional (#153)
19dcd15 
`foo` will always be replaced by the corresponding `three.js` instance, even if it's not being used by the animation logic itself (which is unlikely but possible).
vatro pushed a commit that referenced this issue Nov 23, 2022
Scene Refactor / fix env_tex prop handling (#153)
d72d9b3
vatro pushed a commit that referenced this issue Nov 23, 2022
SvelthreeAnimationManager Suppress development logs (#153)
71bb525 
Change logging from `debug` to `info` where it makes more sense, warn only where really needed / critical., log errors.
vatro pushed a commit that referenced this issue Nov 23, 2022
PropUtils Refactor logs (add SVELTHREE) and suppress `setLookAtFr…
d6ddb4a 
…omValue` warning (#153)
vatro pushed a commit that referenced this issue Nov 23, 2022
SvelthreeProps Change warning message (#153)
327f2ae
@vatro
Copy link
Owner Author

vatro commented Nov 23, 2022 •
edited
Loading

Current Status

svelte-check: OK ✔️ (no warnings / errors)
eslint: OK ✔️ (no warnings / errors)

So far, all "new" test-scenes are working as expected,

Next I need to adapt the rest of the old test-scenes + add some more, this will take a couple of days.
Will create new issues if I encounter some problems.

@vatro vatro mentioned this issue Nov 23, 2022
Merged
@vatro vatro unpinned this issue Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vatro vatro

Labels
code quality refactor
Projects
None yet
Milestone
1.0.0-next.1
Development

Successfully merging a pull request may close this issue.

Refactor everything in strict mode vatro/svelthree
1 participant
@vatro