fix(cli): properly handle errors

GeorgeTaveras1231 commented 11 months ago

Handle all errors centrally and properly set process exit code
Use console.warn where appropriate to funnel messages to stderr
Do not ignore errors when loading config file

Details

I decided to open this PR after noticing some subtle issues in my build pipeline. I noticed that the typed-scss-modules build was silently failing - it couldn't load the config file because I had a broken import inside of it. Additionally, it continue trying to generate types after not being able to load the config, and it encountered another issue - it wasn't able to generate the types because it could not properly parse the sass files (because the configs were broken).

Example error:

[ typed-scss-modules ]: An error occurred loading the config file "<ROOT>/typed-scss-modules.config.js":
[ typed-scss-modules ]: ReferenceError: creaeSassEnhancedImporter is not defined
[ typed-scss-modules ]: Found 4 files. Generating type definitions...
[ typed-scss-modules ]: Can't find stylesheet to import.
[ typed-scss-modules ]:   ╷
[ typed-scss-modules ]: 1 │ @forward 'some-pkg' as some-pkg-*;
[ typed-scss-modules ]:   │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[ typed-scss-modules ]:   ╵
[ typed-scss-modules ]:   src/__tests__/fixtures/forwarded-symbols/b.scss 1:1  @forward
[ typed-scss-modules ]:   src/__tests__/fixtures/forwarded-symbols/a.scss 1:1  root stylesheet (<ROOT>/__packages/docusaurus-sassdoc-plugin/src/__tests__/fixtures/forwarded-symbols/b.scss[1:1])
[ typed-scss-modules ]: Can't find stylesheet to import.
[ typed-scss-modules ]:   ╷
[ typed-scss-modules ]: 1 │ @forward 'some-pkg' as some-pkg-*;
[ typed-scss-modules ]:   │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[ typed-scss-modules ]:   ╵
[ typed-scss-modules ]:   src/__tests__/fixtures/forwarded-symbols/b.scss 1:1  root stylesheet (<ROOT>/__packages/docusaurus-sassdoc-plugin/src/__tests__/fixtures/forwarded-symbols/b.scss[1:1])

TODO

[x] Expose errors from sass.renderSync

Additional changes

Set the TS target to 2022 to support the second argument of the Error constructor. This should be a harmless change, as the cause option is supported in Node 16, but the TS types are a bit delayed (https://github.com/microsoft/TypeScript/issues/49535)

GeorgeTaveras1231 commented 10 months ago

@skovy I've fixed the lint check. Let me know if I should fix anything else.

skovy commented 10 months ago

should this be considered a breaking change? if some peoples builds are technically passing today, this could cause them to start failing?

GeorgeTaveras1231 commented 10 months ago

@skovy that's a good question - I would leave it up to you to decide ultimately. In the past I've chosen both of the options you have (push as a breaking change, or push as a patch).

Its totally reasonable to push this as a breaking change and it is the safer path. But also totally justifiable to push this as a patch -- suggesting that these issues weren't intended to be in the current major.

I'm fine with either approach 😌

skovy commented 10 months ago

My concern with merging this right now is that it sounds like without first finding a solution in https://github.com/skovy/typed-scss-modules/pull/220, this will start to error for people without a fix?

I think it makes most sense if we first merge a solution to the other problem, then this PR?

GeorgeTaveras1231 commented 10 months ago

@skovy that's totally reasonable. Feel free to respond here or on the other PR but remind me what is left for the #220 to be considered mergeable? I still owe you some test cases 😌. And I responded to your last comment about the approach.

skovy / typed-scss-modules