Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS-329 Upgrade TypeScript to 5.6.2 #4816

Merged
merged 1 commit into from
Sep 18, 2024
Merged

JS-329 Upgrade TypeScript to 5.6.2 #4816

merged 1 commit into from
Sep 18, 2024

Conversation

saberduck
Copy link
Contributor

@saberduck saberduck commented Sep 12, 2024

Behavior wrt. project references has changed. Previously, if a project overlapped with the referenced project, it would include the files of the referenced project regardless. Now, the files from the referenced project are not included. This is better for us since it avoids analyzing the files duplicitly.

@saberduck saberduck requested a review from a team September 12, 2024 14:52
@pmaieref
Copy link

Hi, I was about to open a thread in the forum regarding the TypeScript update, but as I see this here now, I thought I might just ask here. If it is too off-topic, please tell me and I'll move it to the community forum.

I am wondering if there are any guidelines, processes, documentation on how TypeScript versions are handled within SonarQube / SonarJS. The documentation does not provide any insights into:

  • Can my local project use a higher / lower TS version than SQ/SonarJS
    • Considering this PR makes it into SQ 10.7, do I need my project on TS 5.6 as well to work in SQ 10.7?)
  • What is the release cadence of TS updates in SonarJS? I see that 5.5 was skipped and you update directly to TS 5.6. Considering TS 5.6 is now 4 days old, this update is "early" (thanks for that!), but "support" for TS 5.5 is quite late.
  • How do certain TS compiler flags play into this? Does SQ need to explicitly support / drop support for them?

The project I am working on, strives to use current / up-to-date versions of Angular, but in the past SQ's TS version was a blocking issue that prevented updating Angular as they raise the minimum versions of TS every 6 months with every major update. We ran into (compilation / execution) issues due to different TS versions and had to downgrade again. Now we don't even try if it would maybe work and just wait it out.

Thanks in advance.

@gabriel-vivas-sonarsource

Hi @pmaieref,

Indeed, I think this would be better to discuss in the Community.
I'll still provide an answer here while the PR is open :-)

We do regular upgrades as they come. However, sometimes conflicting dependencies prevent us from updating faster.

You should be fine using older versions of TypeScript. We don't officially support newer versions. You might experience issues with new compiler options, which can be solved with a dedicated tsconfig.sonar.json. In other cases, we won't yet support new syntax in files that use it, but the rest of your analysis might work well. YMMV.

Keep in mind that if you are using SonarQube, you'll only get the updates when SonarQube itself is released and you update.
The SonarQube releases happen every ~2 months (latest version) or every ~18 months (long-term version).

Hope that helps!

@pmaieref
Copy link

@gabriel-vivas-sonarsource Highly appreciated the reply. If needed I'll create a post in the forum for any further discussions. Yes, we keep our SQ always on latest, thanks. :)

Maybe you could discuss internally if a small update to https://docs.sonarsource.com/sonarqube/latest/analyzing-source-code/languages/javascript-typescript-css/ would make sense, i.e. adding some information regarding TS version support, tsconfig.sonar.json, etc..

Thank you very much for the quick reply here and good luck with the update! :)

@ilia-kebets-sonarsource
Copy link
Contributor

@saberduck What are all the ruling diffs? Is there a general pattern?

@ilia-kebets-sonarsource
Copy link
Contributor

It seems that all ruling issues disappeared from file packages/vuetify/src/components/VDatePicker/VDatePickerDateTable.ts

Copy link
Contributor

@ilia-kebets-sonarsource ilia-kebets-sonarsource left a comment

Choose a reason for hiding this comment

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

There are changes in the ruling for these rules which depend on the TS type checker:

and weirdly one issue gone for rule S6606: Nullish coalescing should be preferred in this location

and one weirder thing: All issues are gone from the vuetify:packages/vuetify/src/components/VDatePicker/VDatePickerDateTable.ts file.

Comment on lines 1 to 5
{
"vuetify:packages/vuetify/src/components/VDatePicker/VDatePickerDateTable.ts": [
null
]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What is null here?

Copy link
Contributor

Choose a reason for hiding this comment

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

see #4816

Copy link
Contributor

Choose a reason for hiding this comment

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

If you like the changes here, you should apply them to all the packages/ruling/tests/projects/*.ruling.test.ts files

@saberduck
Copy link
Contributor Author

It seems that all ruling issues disappeared from file packages/vuetify/src/components/VDatePicker/VDatePickerDateTable.ts

It seems that on Mac, we reached Maximum call stack size exceeded in file vuetify:packages/vuetify/src/components/VDatePicker/VDatePickerDateTable.ts. I had to pass --stack-size=1000 for a ruling to pass on my Mac machine. On CI, the limit is probably higher (Linux). I will fix the CI in another PR so the failure is easier to debug.

Copy link

@saberduck saberduck merged commit cb5ca05 into master Sep 18, 2024
16 of 17 checks passed
@saberduck saberduck deleted the tibor/ts56 branch September 18, 2024 11:56
zglicz pushed a commit that referenced this pull request Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants