bug: TypeScript errors not reported in stencil build when components.d.ts is not present

[ethanbdev]

Prerequisites

• I have read the Contributing Guidelines.
• I agree to follow the Code of Conduct.
• I have searched for existing issues that already report this problem, without success.

Stencil Version

v2.17.3 (Seen in at least 2.16.1 as well)

Current Behavior

Type errors are not erroring on initial builds (no build outputs), but will on subsequent builds.

This is an issue especially in CI, as we re-create the entire environment. The CI workflow will not error if a type error is committed.

Expected Behavior

The type error should cause the build to error

Steps to Reproduce

1. Clone the stencil starter
2. Delete components.d.ts
3. Change the exports in index.ts to be named or export * from ./components or delete index.ts
4. Make a type error somewhere
5. npm run build -> build completes with no errors

OR

1. In a project that does not commit the components.d.ts, simply delete it, and note that the type error will not be reported.
   (I have provided a repo that matches our setup)

Code Reproduction URL

https://github.com/ethanbdev/stencil-typings-error-repro

Additional Information

It seems extremely breaking that we cannot rely on the types generated in an initial build.

One additional thing I noticed, in the stencil starter if the components.d.ts gets deleted, the build does not even work with a different error about the ./components file is not found

[rwaskiewicz]

Hey @ethanbdev 👋

If I'm reading this correctly, it seems the scenarios in the "Steps to Reproduce" section both require that components.d.ts to be absent. Is that understanding correct? If so, may I ask why the type declaration file isn't available?

This is an issue especially in CI, as we re-create the entire environment. The CI workflow will not error if a type error is committed.

Seems to make it sound like the file isn't checked in to source control - is that the case?

[ethanbdev]

Hey @ethanbdev 👋

If I'm reading this correctly, it seems the scenarios in the "Steps to Reproduce" section both require that components.d.ts to be absent. Is that understanding correct? If so, may I ask why the type declaration file isn't available?

This is an issue especially in CI, as we re-create the entire environment. The CI workflow will not error if a type error is committed.

Seems to make it sound like the file isn't checked in to source control - is that the case?

Hey, thanks for the quick reply.. Since it is an autogenerated file it does not make sense to me to check it into source control. Is it a recommended best practice to check that file in? It would change with nearly every pull request and the developer would have to make sure to commit it.

[rwaskiewicz]

Since it is an autogenerated file it does not make sense to me to check it into source control. Is it a recommended best practice to check that file in?

It is recommended this file be checked in at this time. Stencil currently requires this file to be present to properly perform type checking. We're aware this isn't desirable for everyone, and are tracking issues related to this in #3239.

[rwaskiewicz]

For now, I've opened a PR in our documentation to make it explicit we recommend this file be checked in stenciljs/site#907

[ethanbdev]

Since it is an autogenerated file it does not make sense to me to check it into source control. Is it a recommended best practice to check that file in?

It is recommended this file be checked in at this time. Stencil currently requires this file to be present to properly perform type checking. We're aware this isn't desirable for everyone, and are tracking issues related to this in #3239.

Okay, thank you for the update and thanks for updating the doc. We can monitor that issue. Should I add some of this info as a comment there?

[rwaskiewicz]

Appreciate the offer! I think we have everything we need - when I mentioned it above, that gave us a good link to this issue like so in 3239:

I'm gonna keep this issue open and label it so it gets into the backlog. I want to make sure this case (although its pretty fundamental to the changes we'd make) gets tested/kept in mind.

[PMudra]

If I'm reading this correctly, it seems the scenarios in the "Steps to Reproduce" section both require that components.d.ts to be absent. Is that understanding correct? If so, may I ask why the type declaration file isn't available?

I am facing the described issue while having components.d.ts checked into git.

It seems to me that TypeScript errors are also not reported if the components.d.ts file is modified during the build.

This is what happened: We are automatically merging minor and patch npm package updates using renovate bot. These updates are usually protected by a build pipeline. A pipeline will run and block the merge request if bundling, TypeScript or tests fail.

One of the latest Stencil updates led to a modified components.d.ts file. Some code comments were added to the file that are taken from component classes (I can try to find the exact commit if needed; nice feature!). This update was safely merged into our codebase because it was only a minor or patch version and the pipeline passed.

What we did not notice is that this Stencil update disabled the TypeScript error reporting for all future updates done by the bot. That is because every build would generate a modified version of components.d.ts which somehow disables TypeScript.

This behavior is not expected and is somehow difficult to handle. I am thinking about these workarounds:

1. Let the pipeline fail after the build components.d.ts is modified. Or even if any file is modified (that is not ignored by .gitignore).
2. Disable automatic Stencil updates.

I guess these workarounds would not be necessary if #3239 is fixed.

[danyball]

I can confirm what @PMudra wrote:
We have not checked in "components.d.ts".

1. running stencil test --e2e
   --> no TypeScript errors although we have unused imports
   --> components.d.ts is created

2. after that, running stencil build
   --> components.d.ts has changed (!) (because we have a weird import in a component's file referencing the components.d.ts file)
   --> no TypeScript errors although we have unused imports

[Manipandian]

@rwaskiewicz, Any update on this issue?
We are running stencil build two times in pipeline to avoid this discrepancy on type checking. And its causing notable delay on each build generation?

[christian-bromann]

@Manipandian there are no updates to this case. The Stencil team has many competing priorities and limited resources to give every issue the necessary attention it deserves. At this point I recommend to get involved providing a fix for this issue as I can guarantee when we will be able to take a look at this. We are happy to provide guidance in the process. Thanks for understanding.