Vite 5 and updated all packages with eslint flat config

TechAkayy commented 1 month ago

Description 📖

This pull request updates Iles & it's packages to Vite 5 along with all it's dependencies (had partial success, but with help we can fix it).

Commit-1 - Updated dependencies along with any API changes in their major versions (not many). Also, updated to eslint 9 with a flag config eslint.config.mjs. Installed @vue-macros/reactivity-transform to keep $-style APIs.

Commit-2 - Minor types references updated in two packages' package.json.

Commit-3/4 - Removed @vue-macros/reactivity-transform and updated to standard vue APIs. This can be ignored if you want to keep the $-style APIs.

There are still some minor hiccups for which I need help. Details with screenshots below.

Screenshots 📷

pnpm install installs without any peer dependency errors except for @mussi-eslint-config related. I guess this is your package @ElMassimo, and wondering what's the purpose of it? Is it still required in the new eslint 9 flat config way?

pnpm build:all builds fine.

pnpm dev throws some types warnings, looks harmless.

The reason I removed reactivity-transform macro is because there seems to be some issue with it. Notice the missing semicolon after .../BackTick.tsx'. It was turned into a blocker for me.

Key Issue:

Client Script Block seem to not work. When used,

<script client:load lang="ts">
console.log('Powered by îles 🏝', 'https://iles-docs.netlify.app')
</script>

I could trace it to an error logged by vue, something around missing end tag.

I have added three // @ts-ignore in https://github.com/TechAkayy/iles/blob/417c641586484ede92f1bf12579d517479294703/packages/iles/src/node/plugin/wrap.ts#L50 which is related to the possibility of template.ast to be undefined. Type wasn't reconciling as ast showed up as RootNode, while some functions that it was pass through was expecting ElementNode.

Traced reference to this vue/core update https://github.com/vuejs/core/commit/6fab8551e4aeef4610987640de8b435b1ae321bb.
Related updates in @vitejs/plugin-vue - https://github.com/vitejs/vite-plugin-vue/commit/ba1bab977d51a341296c474237393a58237045c8, https://github.com/vitejs/vite-plugin-vue/commit/bc0ad64e9047c3991b4be5a3f41e48bdb63ca2d6, https://github.com/vitejs/vite-plugin-vue/commit/5d68fbdaa626f70a3763cb611eee1c2d22be4d5e & https://github.com/vitejs/vite-plugin-vue/commit/36351fafb69020ec18301bd7ae3cd3f15c422f31.

TechAkayy commented 1 month ago

One important thing - because of the peer deps issue with @mussi/eslint-config, I couldn't lint properly.

ElMassimo commented 1 month ago

Hi Ahmed!

Appreciate you sharing this pull request.

Regarding the ESLint config, I'd be ok using something like @antfu/eslint-config and tweak a few rules which align more with standardjs.

For now feel free to ignore any lint issues, or disable that workflow.

I'll follow up on this when I have some time, thanks!

TechAkayy commented 1 month ago

Completed my PR for the update, here is a quick summary:

Configured @antfu/eslint-config (thanks @antfu) and removed @mussi/eslint-config. Main package.json has a minor addition to allow eslint9 until the time rest of the ecosystem catchup, see Anthony Fu's comment here. Added these rules to the flat config as these are generously exempted in the current codebase.
```
'unused-imports/no-unused-vars': 'off',
'style/max-statements-per-line': 'off',
'no-console': 'off',
'node/prefer-global/process': 'off',
'antfu/no-import-dist': 'off',
```
Added lint:fix to all packages, and completed lint fixes for the whole codebase (in a seperate commit to track). For errors that required manual intervention, I have applied non-destructive eslint-disable-* for specific lines so that the code doesn't change in any way (unless types changed in major version of dependencies). We should ideally remove these going forward.
Updated pnpm to latest packageManager (thanks for the reminder @userquin). You might encounter this pnpm issue that might look like a vite type (PluginOption) issue.
Vite 5 breaking change - manifest files are now within a .vite subfolder within build.outDir.
Vite 5 - enabled cssCodeSplit in client bundles without which css wasn't getting extracted.
Vue 3.4 breaking change - template ast had a type change from ElementNode to RootNode (this spot) as part of performance enhancement - details here. This required wrap logic to be refactored (the three @tsignores in my original above post) as we use this internal api in packages/iles/src/node/plugin/wrap.ts
Vue 3.4 - Though not documented, custom blocks naming has to be in kebab-case, so changed scriptClient to script-client in Vite transform hook, without which vue core compiler throws an error (screenshot in my first post above). The validation probably was added later in the underlying compiler.
Both docs & blog repos dev & built nicely (windicss version)
Migrated the docs & blog to Unocss using Wind preset, dev & built nicely.

Only thing I couldn't solve is - in the blog, the one-piece image doesn't seem to be optimised, probably related to the underlying vite plugin, or in its usage (was there any breaking changes @ElMassimo?) - https://github.com/ElMassimo/vite-plugin-image-presets.

For the users updating iles, in their apps, with this update, they will have to:

remove version pinning of vue & vite in their package.json (if applicable). TODO: We might need to add vue & vite latest as peer deps as part of the update?
add type: module to their package.json and manage any uncompatible commonjs vite plugins if they show trouble, as vite 5 is esm-first
breaking: migrate off reactivity-transform ($computed -> computed with .value on refs, etc) or use the macro.

Might be worth spinning out a prerelease verison for our users to test after your review & if it works nicely at your end. I did test on a few samples at my end, worked nicely :smile: Thanks 👍

TechAkayy commented 1 month ago

With Vite 5 deprecating cjs node api, I have refactored tryInstallModule to tryImportOrInstallModule and use dynamic import() which works with both cjs & esm modules even when a package doesn't have main or exports defined in it's package.json. This takes care of packages like the svelte vite plugin that is esm only.

Also, antfu's install-pkg uses ezspawn which is not well maintained, and is currently broken with one of it's downstream package. So, I have taken inspiration from that and have used a childprocess exec to achieve the same in a simple way, we still use that package for detecting package manager. Auto install of iles modules works smooth with this, and I can see the installation succeeding, but testing on a local project (outside the playground folder) by pnpm linking the iles repo has been hard (probably a pnpm + monorepo challenge like in this issue).

ElMassimo commented 2 days ago

Hi @TechAkayy!

Would you enable me to push updates to this branch in your fork?

TechAkayy commented 2 days ago

Hi @TechAkayy!

Would you enable me to push updates to this branch in your fork?

Just invited you @ElMassimo. Thanks!

Good idea creating a fork without linting. Please let me know if you require anymore follow-ups from me. Cheers!

ElMassimo / iles