maizzle / framework

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

Maizzle doesn't report any errors with tailwind.config.js #871

Closed adrian-sgn closed 8 months ago

adrian-sgn commented 1 year ago

If the local tailwind.config.js file contains any errors, the code to require that config file fails silently and defaults to using the built-in tailwind.config.js file, leaving me as a developer confused why my tailwind config isn't working.

This is happening here, by design: https://github.com/maizzle/framework/blob/master/src/generators/tailwindcss.js#L22-L30

      /**
       * Try loading a fresh tailwind.config.js, with fallback to an empty object.
       * This will use the default Tailwind config (with rem units etc)
       */
      try {
        return requireUncached(path.resolve(process.cwd(), tailwindConfig))
      } catch {
        return {}
      }

Which uses the requireUncached() function from utils/helpers.js which masks any caught exception as simply "Could not load ${module}"

  requireUncached: module => {
    try {
      delete require.cache[require.resolve(module)]
      return require(module)
    } catch {
      throw new Error(`could not load ${module}`)
    }
  },

Given how central the tailwind config is to the framework, and how detailed it can become, as a developer I expect if I have errors in my tailwind config that Maizzle would bubble up the exception so I can debug, including both cases for when the file has errors and if the file doesn't exist. (ex. I made a typo in my Maizzle build.tailwind.config setting)

Within the framework, it appears that requireUncached is only used in two places: generators/config.js and generators/tailwindcss.js. It seems reasonable that the try .,. catch could be removed from this function.

To allow the underlying errors to bubble up, the try... catch would also need to be removed from tailwindcss.js where requireUncached is called. I don't understand the reason why those errors need to be trapped and ignored there to just remove those clauses.

cossssmin commented 1 year ago

Hi there, thanks for reporting this.

You're right, when the config can't be loaded we should throw instead of returning an empty object (which makes Tailwind use its default config). This would be a breaking change though, so it will have to come in the next major release.

sattes-faction commented 1 year ago

I agree, it's currently hard to track down errors, if errors are silently swallowed. Maybe you can also add some output, which mentions which config it tries to load or if it falls back to a default one without having to refactor everything.

Something like a verbose or debug mode would help as well. I'm aving a hard time to find out, why Maizzle doesn't like certain things and ended up adding console.logs in the maizzle code itself ;-)

cossssmin commented 8 months ago

In Maizzle 5 we're not using a custom Tailwind compiler anymore, so if Tailwind throws an error it you'll see it 👍