Skip to content

fix: Do not perform type checking for TS files from jar files#15700

Merged
mshabarov merged 2 commits into
masterfrom
no-ts-check-jar-resources
Feb 2, 2023
Merged

fix: Do not perform type checking for TS files from jar files#15700
mshabarov merged 2 commits into
masterfrom
no-ts-check-jar-resources

Conversation

@Artur-

@Artur- Artur- commented Jan 17, 2023

Copy link
Copy Markdown
Member

The TS files that come from JAR files are not under control of the project developer so enforcing project TS rules on them does not make sense. This is similar to "skipLibCheck" used for TS files that are in node_modules.

Fixes #15485

@github-actions

github-actions Bot commented Jan 17, 2023

Copy link
Copy Markdown

Test Results

   942 files  ±0     942 suites  ±0   56m 3s ⏱️ - 2m 22s
5 898 tests +1  5 863 ✔️ +1  35 💤 ±0  0 ±0 
6 152 runs  +3  6 110 ✔️ +3  42 💤 ±0  0 ±0 

Results for commit 1ff951e. ± Comparison against base commit 895c1ee.

♻️ This comment has been updated with latest results.

@@ -47,8 +47,8 @@ public class TaskGenerateTsConfig extends AbstractTaskClientGenerator {
private static final String VERSION = "flow_version";
private static final String ES_TARGET_VERSION = "target";
private static final String TSCONFIG_JSON_OLDER_VERSIONS_TEMPLATE = "tsconfig-%s.json";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6% of developers fix this issue

InlineFormatString: Prefer to create format strings inline, instead of extracting them to a single-use constant


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@sonatype-lift

sonatype-lift Bot commented Jan 19, 2023

Copy link
Copy Markdown

🛠 Lift Auto-fix

Some of the Lift findings in this PR can be automatically fixed. You can download and apply these changes in your local project directory of your branch to review the suggestions before committing.1

# Download the patch
curl https://lift.sonatype.com/api/patch/github.com/vaadin/flow/15700.diff -o lift-autofixes.diff

# Apply the patch with git
git apply lift-autofixes.diff

# Review the changes
git diff

Want it all in a single command? Open a terminal in your project's directory and copy and paste the following command:

curl https://lift.sonatype.com/api/patch/github.com/vaadin/flow/15700.diff | git apply

Once you're satisfied commit and push your changes in your project.

Footnotes

  1. You can preview the patch by opening the patch URL in the browser.

@Artur-

Artur- commented Jan 20, 2023

Copy link
Copy Markdown
Member Author

Seems like this does not actually fix the issue. Files which are included in other files are checked even with the exclude section

@Artur- Artur- closed this Jan 20, 2023
@Artur- Artur- reopened this Jan 31, 2023
@vaadin-bot vaadin-bot added +1.0.0 and removed +0.0.1 labels Jan 31, 2023
@Artur- Artur- force-pushed the no-ts-check-jar-resources branch from a687650 to 81509d8 Compare January 31, 2023 12:41
The TS files that come from JAR files are not under control of the project developer so enforcing project TS rules on them does not make sense. This is similar to "skipLibCheck" used for TS files that are in node_modules.

Fixes #15485
@Artur- Artur- force-pushed the no-ts-check-jar-resources branch from 81509d8 to 2029518 Compare January 31, 2023 12:42
@vaadin-bot vaadin-bot added +0.0.1 and removed +1.0.0 labels Jan 31, 2023
@Artur- Artur- requested a review from mshabarov January 31, 2023 16:29
@sonarqubecloud

sonarqubecloud Bot commented Feb 1, 2023

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mshabarov mshabarov merged commit 10019fe into master Feb 2, 2023
@mshabarov mshabarov deleted the no-ts-check-jar-resources branch February 2, 2023 08:22
vaadin-bot pushed a commit that referenced this pull request Feb 2, 2023
The TS files that come from JAR files are not under control of the project developer so enforcing project TS rules on them does not make sense. This is similar to "skipLibCheck" used for TS files that are in node_modules.

Fixes #15485
Artur- added a commit that referenced this pull request Feb 2, 2023
The TS files that come from JAR files are not under control of the project developer so enforcing project TS rules on them does not make sense. This is similar to "skipLibCheck" used for TS files that are in node_modules.

Fixes #15485
vaadin-bot added a commit that referenced this pull request Feb 2, 2023
… (CP: 23.3) (#15803)

* fix: Do not perform type checking for TS files from jar files (#15700)

The TS files that come from JAR files are not under control of the project developer so enforcing project TS rules on them does not make sense. This is similar to "skipLibCheck" used for TS files that are in node_modules.

Fixes #15485

* add import

---------

Co-authored-by: Artur <artur@vaadin.com>
@vaadin-bot

Copy link
Copy Markdown
Collaborator

This ticket/PR has been released with Vaadin 24.0.0.alpha10 and is also targeting the upcoming stable 24.0.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stricter TS checks in Vaadin 23.3.0

3 participants