maizzle / framework

Quickly build HTML emails with Tailwind CSS.
https://maizzle.com
MIT License
1.2k stars 48 forks source link

Minor issues with config types on v5 beta10 #1303

Closed d3v1an7 closed 1 month ago

d3v1an7 commented 1 month ago

Hey! Just upgraded to 5 and noticed maizzle.config.js was complaining. I'm using VSCode and my project is vanilla JS but with jsconfig.json and "checkJs": true.

Original code

/** @type {import('@maizzle/framework').Config} */
export default {
  build: {
    components: {
      folders: [
        'netlify/functions/foo/config/components',
      ],
    },
  },
  inlineCSS: true,
  removeUnusedCSS: true,
  shorthandCSS: true,
  minify: true,
};

Error

Type '{ build: { components: { folders: string[]; }; }; inlineCSS: boolean; removeUnusedCSS: boolean; shorthandCSS: boolean; minify: true; }' is missing the following properties from type 'Config': css, filters, beforeCreate, beforeRender, and 3 more.ts(2740)

Fix

I checked the type definitions and just added empty/no-op stuff for now:

/** @type {import('@maizzle/framework').Config} */
export default {
  build: {
    components: {
      folders: [
        'netlify/functions/generate-newsletter-background/config/components',
      ],
    },
  },
  inlineCSS: true,
  removeUnusedCSS: true,
  shorthandCSS: true,
  minify: true,
  // NOTE: The following should be no-op, they were added to stop Typescript checking from complaining.
  beforeRender: async (html, config, render) => {
    return html;
  },
  afterRender: async ({ html, config }) => {
    return html;
  },
  css: {},
  filters: {},
  beforeCreate: async (config) => {},
  afterTransformers: async ({ html, config, render }) => {
    return html;
  },
  afterBuild: async ({ files, config, render }) => {},
};

Additional note

I noticed the code example for beforeCreate has the params wrapped in {}:

   * @example
   * ```
   * export default {
   *   beforeRender: async ({html, config, render}) => {
   *     // do something...
   *     return html;
   *   }
   * }

But the actual type in events.d.ts is missing the wrapping {}

export type beforeRenderType = (html: string, config: object, render: (html: string, config: object) => Promise<string>) => Promise<string>;
cossssmin commented 1 month ago

Hey thanks for this, I actually updated the types in beta.12 yesterday. Working on making a small update there again and will release beta.13 shortly (wanted to expose transformers in events but that's not really useful since they can all just be imported anyway).

cossssmin commented 1 month ago

Hmm just noticed build.components is mistyped, this has moved to the root of the config so you'd do:

/** @type {import('@maizzle/framework').Config} */
export default {
  components: {
    folders: [
      'netlify/functions/foo/config/components',
    ],
  },
}

Will fix that one as well in beta.13.

cossssmin commented 1 month ago

https://github.com/maizzle/framework/releases/tag/v5.0.0-beta.14

cossssmin commented 1 month ago

Actually can you share your jsconfig.json? Just tried it and not getting autocomplete anymore, must be something I'm configuring wrong.

d3v1an7 commented 1 month ago

Sure thing! But fair warning, this is my first dive into this stuff, so if something looks off or weird, that's probably me misunderstanding something :)

{
  "compilerOptions": {
    "target": "es2017",
    "module": "esnext",
    "moduleResolution": "bundler",
    "lib": ["dom", "es2022"],
    "checkJs": true,
    "resolveJsonModule": true
  },
  "include": ["global.d.ts", "src/**/*", "netlify/functions/**/*"],
  "exclude": ["node_modules"]
}
cossssmin commented 1 month ago

Hehe no worries I'm really not into TS myself, just trying to provide some nice DX for the config through these type defs :)

Thanks, I'll test it out and see if there's anything I can do šŸ‘

d3v1an7 commented 1 month ago

5.0.0-beta.14 solves a bunch of the issues above -- but I'm wondering if in config.d.ts, does it make sense for the following to be optional?

I'm not sure about the internals and what is actually mandatory for Maizzle to run, but perhaps even also:

cossssmin commented 1 month ago

Yes you're right those should all be optional, will fix that sorry šŸ‘

cossssmin commented 1 month ago

https://github.com/maizzle/framework/releases/tag/v5.0.0-beta.15