Refactor everything :) #26
Conversation
bezoerb
force-pushed
the
feature/typescript
branch
from
e05e6d4
to
84cc060
Compare
bezoerb
force-pushed
the
feature/typescript
branch
from
ea03db9
to
9862663
Compare
…data available everywhere
| "main": "./dist/index.js", | ||
| "type": "module", | ||
| "typings": "./dist/types.d.ts", | ||
| "exports": { |
There was a problem hiding this comment.
Da sind ja einige Dateien drinne.
Zusammen mit den typesVersions frage ich mich ob man das nicht vereinfachen kann, indem man die logik aus der index in eine weitere datei schiebt und in der index ein objekt exportiert was die hier genannten sachen enthält?
There was a problem hiding this comment.
ich will das ungern alles in einem großen object drin haben...
ggf. will man ja auch nur z.B. den toml converter oder den object helper importieren.
Dann finde ich das so schöner:
import {snakeCaseKeys} from '@jungvonmatt/contentful-ssg/helper/object';
import toml from '@jungvonmatt/contentful-ssg/converter/toml';There was a problem hiding this comment.
So viele interne export Punkte können aber schnell zum Problem werden, wenn man was refactoren will.
Das sind ja quasi alles Schnittstellen die man genau in der Struktur nach außen gibt und die sich nicht ändern dürfen wenn man kompatibel bleiben woll.
Vielleicht statt einem großem Objekt dann mehrere kleine, aber auf oberster Ebene? (index, helper, transformer, ...)?
Das sind dann nur Typisierte Hüllen. Ich. meine sowas habe ich schon mal woanders gesehen.
There was a problem hiding this comment.
ich check das nochmal ;)
|
|
||
| ### Runtime Hooks | ||
| **before** | ||
| ```js |
There was a problem hiding this comment.
Da wir typescript unterstützen wäre es doch super, wenn wir hier die Beispiele in Typescript schreiben.
Wer JavaScript nutzen will muss die Typen dann einfach weglassen.
Würde das ganze meiner Meinung nach verdeutlichen.
There was a problem hiding this comment.
Finde ich schwierig... Da man durch den Typ auch nicht mehr weiß.
In der Readme gibt's leider kein type hinting.
Und die Funktion zu typisieren wäre dann mit Parameter Types und Allen möglichen Return Types schon etwas unübersichtlich finde ich:
z.B. bei einem Transform Hook:
(transformContext: TransformContext, runtimeContext: RuntimeContext, prev: KeyValueMap): KeyValueMap | Promise<KeyValueMap> => {
...
}| return [...this.ignoredFiles].length; | ||
| } | ||
|
|
||
| get ignoredFiles() { |
There was a problem hiding this comment.
Ich fände es übersichtlicher, wenn alle Funktionen einen Return Type definieren müssen.
Ausnahme wäre nur eine Funktion die nur void zurück gibt.
Die Regel ist bei XO leider deaktiviert
There was a problem hiding this comment.
Der return type ergibt sich doch an sich automatisch aus dem context
| errors: StatsEntry[] = []; | ||
| skipped: StatsEntry[] = []; | ||
|
|
||
| constructor(config: Config) { |
There was a problem hiding this comment.
Mit dem aktuellen Typescript kann man schreiben
constructor(private readonly config: Config) {}Da lässt man die config field Deklaration oben auch weg.
There was a problem hiding this comment.
dann kann ich die aber in den tests nicht mehr ändern, oder?
| tsconfigRootDir: __dirname, | ||
| extraFileExtensions: [".cjs"] | ||
| }, | ||
| ignorePatterns: ['**/*.cjs','src/**/*.test.ts', 'src/__test__/*',"src/**/*.fixme.ts"], |
There was a problem hiding this comment.
Ich finde auch Tests sollten sich an Linter Settings halten.
There was a problem hiding this comment.
Dann müsste ich die auch in der tsconfig.json freischalten
Parsing error: "parserOptions.project" has been set for @typescript-eslint/parser.
The file does not match your project config: src/tasks/setup.test.ts.
The file must be included in at least one of the projects provided.
kann man später nochmal nachziehen ;)
growpreset to pluginsI had to disable tests for the grow plugin as ts-jest doesn't seem to play nice with the monorepo/esm construct