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

feat(app-vite&app-webpack): upgrade tsconfig preset #17301

Merged
merged 5 commits into from
Aug 1, 2024

Conversation

IlCallo
Copy link
Member

@IlCallo IlCallo commented Jun 20, 2024

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Documentation
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

Minumum accepted TS version would be 5, since we'll use bundler option for moduleResolution

The PR fulfills these requirements:

  • It's submitted to the dev branch (or v[X] branch)
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
  • It's been tested on a Cordova (iOS, Android) app
  • It's been tested on an Electron app
  • Any necessary documentation has been added or updated in the docs or explained in the PR's description.

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to start a new feature discussion first and wait for approval before working on it)

Other information:

I tested Vite new preset on a couple of projects of ours and aside the problems I reported down there, which should be used as base for a migration guide, I didn't have other problems


TS reference suggests to avoid using baseUrl and rely on relative values for path option
This isn't possible in our case, since we ship default "paths" in the presets instead of the devland tsconfig
Using relative paths in the preset would cause the mapping to search types relatively to the preset tsconfig location and not the devland project folder
There doesn't seem to be another way to achieve this feature, and putting the paths back into the devland tsconfig would remove a good chunk of our flexibility to update those paths on the user behalf
Instead, I added comments to explain this where needed
We could remove "baseUrl" from jsconfig or older tsconfig from Qv1 which weren't using a tsconfig preset, but it doesn't really make sense changing something that's not broken


Switching to bundler option for moduleResolution breaks any kind of deep import, since it nows honors exports field in package.json when defined
Deep imports are still possible, but the dev needs to define an appropriate entry in paths option mapping to the deep import location into node_modules folder

E.g.

{
  "compilerOptions": {
    "paths": {
      // ... other Quasar paths definition
      "cypress/types/net-stubbing": ["node_modules/cypress/types/net-stubbing"],
      "@vue/apollo-composable/dist/*": ["node_modules/@vue/apollo-composable/dist/*"]
    }
  }
}

Additionally, bundler mode won't recognize CJS imports, thus any kind of CJS file, especially quasar.conf.js or quasar.conf.cjs in our case, isn't correctly interpreted by TS
I haven't tried, but I expect quasar.conf.ts or quasar.conf.mjs to work with this setup


I added a new version of the presets which is even stricter, and thus could not fit everyone

useDefineForClassFields in Vite tsconfig preset isn't needed, since it's true by default when target is esnext as it is.
This is true since Vite 2.5 apparently

Regarding target option, it's actually ignored by Vite, so setting it to esnext is a good way to force useDefineForClassFields to true and accept any kind of new syntax and feature

For webpack, target is apparently used to determine what to transpile, so I guess we should still keep it set to es6

From TS documentation:

Setting this value to the lowest ECMAScript version that you intend to support ensures the emitted code will not use language features introduced in a later version.

@IlCallo IlCallo changed the title feat(app-vite): upgrade tsconfig preset feat(app-vite&app-webpack): upgrade tsconfig preset Jul 26, 2024
@rstoenescu rstoenescu marked this pull request as ready for review August 1, 2024 10:14
@rstoenescu rstoenescu merged commit 7226961 into quasarframework:dev Aug 1, 2024
1 of 2 checks passed
@IlCallo IlCallo deleted the update-tsconfig-presets branch August 1, 2024 10:23
@MartinX3
Copy link

MartinX3 commented Aug 5, 2024

@rstoenescu I don't have the stricter-tsconfig-preset.json in Beta 16.
I also don't see it here
https://www.npmjs.com/package/@quasar/app-vite/v/2.0.0-beta.16?activeTab=code

Maybe it didn't get released?

@yusufkandemir
Copy link
Member

@MartinX3 thanks for noticing it. It's due to not adding the file to package.json > files.
Fixed in 151d044 and 6b4e280.

@MartinX3
Copy link

MartinX3 commented Aug 6, 2024

@yusufkandemir thank you!
Now we just need another release :D

@rstoenescu
Copy link
Member

Released both q/app-vite betas

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