[WIP] Type fixes #457
[WIP] Type fixes #457
Conversation
.eslintrc.cjs
Outdated
| '../*', // Disallow relative imports that go up a directory | ||
| './*', // Optionally disallow relative imports within the same directory | ||
| '!./*.module.css', // allow relative import of CSS module files | ||
| 'src/*', // Disallow absolute imports starting with `src/` and enforce alias usage |
There was a problem hiding this comment.
I am wondering if we specify ts/js as there may be other files that make sense to import directly other than just module.css
Also, is part of the goal with this change (@engine, etc) to help with plugins or the web export/sub packages, etc?
There was a problem hiding this comment.
So I'm going to remove the rule ./* because I think it's fine to import sibling files. It's just when we start going up and down the file structure with lots of /../ that it gets a bit unwieldy.
The @engine stuff is just for tidiness when importing. For instance rather than having to do src/renderer/components/whatever put the whole import path, we can just do @components/whatever. It also means we could potentially change the location of that folder and all we'd need to update is the alias path. In the case of @engine though this will probably change quite soon when we move it to a monorepo...
There was a problem hiding this comment.
Ok I see your point now, I'm not going to allow neighbouring directories to be imported but neighbouring files (as you suggested) makes sense.
| @@ -95,7 +95,7 @@ export class HedronEngine { | |||
| const sketchInstances = this.sketchManager!.getSketchInstances() | |||
| debugScene.clearPasses() | |||
| Object.values(sketches).forEach((sketch) => { | |||
| // eslint-disable-next-line | |||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |||
| const paramValues: { [key: string]: any } = {} | |||
There was a problem hiding this comment.
Maybe it would be good to get this typed, but I can forsee wanting to have objects like a vec2, vec3, gradient, in the future. Maybe even want to support anything via plugins, and once it's external peoples desires may be endless
There was a problem hiding this comment.
Yeah I'm still not sure how to handle this 🙈. Hopefully we can work out something eventually
|
Merging for now, not sure if it all makes complete sense but we can still change rules if we start getting annoyed by them |
Went through and fixed a bunch of TS issues and linting. The biggest change is forcing imports to always be using aliases. You can never use relative imports. So basically an import always has to start with
@renderer,@main,@engine,@components, etc. We can add more aliases as we go.@cale-bradbury what do you think? Too strict? I'm not even allowing neighbouring imports like
./foo, only exception is./MyComponent.module.css.I'm not too precious over this, just wanted to get some consistency across the codebase, let me know if you hate it! It's worth noting that VSCode's auto import when typing out a variable should automatically use this format so no issues there (let me know if that's the case for you!)