Fix/address left config typing issues #27

casaper · 2020-11-22T18:56:02Z

Add @type definitions for svg2ttf and ttf2woff2
fix generator woff2 to use ttf2woff2 according to type definition
remove these two declarations in "src/types/modules" and add the rest that are still there to the tsconfig.json
replace custom type declarations with @types packages for ttf2eot, ttf2woff and svgicons2svgfont
simplify RunnerOptions to one interface optional properties instead of merging another type with required properties as Optional properties... the concept was just a bit complicated, when typescript actually does optional properties on an interface so well 😉

casaper · 2020-11-24T07:55:04Z

The @types packages were now all merged and the packeges released, so I could now include them as packages. This one is now definitly ready for review

tancredi

Thank you so much for taking the time to get these typings published 🙏 looks good, just a couple comments (I think the type changes to Partial is the most important)

tancredi · 2020-12-08T22:56:13Z

src/constants.ts

 import { FontAssetType, OtherAssetType } from './types/misc';

 export const TEMPLATES_DIR = resolve(__dirname, '../templates');

-export const DEFAULT_OPTIONS: RunnerOptionalOptions = {
+export const DEFAULT_OPTIONS: Partial<RunnerOptions> = {


The reason I had split the optional options was to ensure that as they grew or changed this const definition would break if any of these default options was missing. This looks cleaner, but is less safe (as I could just comment out for example name from defaults)

The Options are not nullable / can't be undefined when they get to the generator, optional just means they come from default values rather than necessarily user input.

Hence Partial doesn't quite serve the same purpose here. Make sense?

It frankly is a bit too far back in time. I don't remember what my reasons were here, and I currently do not have the time to dig into this problem. Perhaps in a week or two.

I replaced the partial with an Omit now: e47c2ac

export const DEFAULT_OPTIONS: Omit<RunnerOptions, 'inputDir' | 'outputDir'> = {

Would it be OK for you like that @tancredi ?

Lots of good work in this PR just wished this was a separate PR as it was a bit off scope here and made it difficult more me to review / merge (sorry for taking lots of time) as I care about keeping option types very very explicit and controllable - reverted just this and made a few additions here and there :)

tancredi · 2020-12-08T23:01:08Z

src/generators/asset-types/__tests__/woff2.ts

-  (({
-    formatOptions: { [FontAssetType.WOFF2]: woffOptions }
-  } as unknown) as FontGeneratorOptions);
+  (({} as unknown) as FontGeneratorOptions);


Why are we removing the woff options mock from tests? They check that the generator options are correctly passed to the font generator.

Hello @tancredi

Fiddling in these @types/ definitions I discovered that ttf2woff2 does not have such options. For unknown rason it used to work, but anyhow, the lib will not process and do anything with it. So I removed the options from the font options type, and this spec conflicted with that fix. It expected the non-existent font option to be passable.

@tancredi

Yes, I verified that now. It is definiteley because I found out taht ttf2woff2 simply does not have any such second param.

See:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/ttf2woff2/index.d.ts#L8

And in TTF2WOFF2 itself:

https://github.com/nfroidure/ttf2woff2/blob/master/jssrc/index.js#L5

No second param there...

That's great to know! I've removed some other unnecessary format options from the types! cheers

…itions and add the rest of the declarations to the tsconfig

And the metadata will be inherited from the ttf anyhow

- no more joins of multiple times and duplicates - the optional properties are real type script optional properties - the templates object allows only template paths for actually possible templateable formats

Replacing the custom declarations for external packages with fresh @types: - @types/svgicons2svgfont - @types/ttf2eot - @types/ttf2woff

…ress-left-config-typing-issues

github-actions · 2021-01-12T20:23:30Z

🎉 This PR is included in version 1.0.31 🎉

The release is available on:

Your semantic-release bot 📦🚀

casaper force-pushed the fix/address-left-config-typing-issues branch from eedda71 to 502b218 Compare November 22, 2020 19:02

casaper requested a review from tancredi November 22, 2020 19:15

tancredi requested changes Dec 8, 2020

View reviewed changes

casaper added 7 commits December 22, 2020 11:19

fix: replace svg2ttf and ttf2woff2 type definitions with @types defin…

52b3b3f

…itions and add the rest of the declarations to the tsconfig

fix(woof2): generator not compatible with @type definition

b2665f9

And the metadata will be inherited from the ttf anyhow

fix(woff-format-options): add correct option with documentation

f495d5d

fix(ttf-generator): add correct format options from type

7867af0

fix(eot-generator): remove format options that do not exist on ttf2eot

b7b547b

make runner options a bit more simple and less confusing

9f73213

- no more joins of multiple times and duplicates - the optional properties are real type script optional properties - the templates object allows only template paths for actually possible templateable formats

replace own lib types with fresh @types packages

e0e692a

Replacing the custom declarations for external packages with fresh @types: - @types/svgicons2svgfont - @types/ttf2eot - @types/ttf2woff

casaper force-pushed the fix/address-left-config-typing-issues branch from 59d58a7 to e0e692a Compare December 22, 2020 10:19

casaper and others added 3 commits December 22, 2020 11:21

fix(constants): default options with omit instead of partial

e47c2ac

Merge branch 'master' of github.com:tancredi/fantasticon into fix/add…

70fe7a0

…ress-left-config-typing-issues

Clean up

1c135c9

tancredi approved these changes Jan 12, 2021

View reviewed changes

tancredi merged commit a4fbc04 into master Jan 12, 2021

tancredi deleted the fix/address-left-config-typing-issues branch January 12, 2021 20:22

github-actions bot added the released label Jan 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/address left config typing issues #27

Fix/address left config typing issues #27

casaper commented Nov 22, 2020 •

edited

Loading

casaper commented Nov 24, 2020

tancredi left a comment

tancredi Dec 8, 2020

casaper Dec 10, 2020

casaper Dec 22, 2020 •

edited

Loading

tancredi Jan 12, 2021

tancredi Dec 8, 2020

casaper Dec 10, 2020

casaper Dec 22, 2020

tancredi Jan 12, 2021

github-actions bot commented Jan 12, 2021

Fix/address left config typing issues #27

Fix/address left config typing issues #27

Conversation

casaper commented Nov 22, 2020 • edited Loading

casaper commented Nov 24, 2020

tancredi left a comment

Choose a reason for hiding this comment

tancredi Dec 8, 2020

Choose a reason for hiding this comment

casaper Dec 10, 2020

Choose a reason for hiding this comment

casaper Dec 22, 2020 • edited Loading

Choose a reason for hiding this comment

tancredi Jan 12, 2021

Choose a reason for hiding this comment

tancredi Dec 8, 2020

Choose a reason for hiding this comment

casaper Dec 10, 2020

Choose a reason for hiding this comment

casaper Dec 22, 2020

Choose a reason for hiding this comment

tancredi Jan 12, 2021

Choose a reason for hiding this comment

github-actions bot commented Jan 12, 2021

casaper commented Nov 22, 2020 •

edited

Loading

casaper Dec 22, 2020 •

edited

Loading