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!: drop CJS and use VueUse v13 #254

Merged
merged 21 commits into from
Mar 10, 2025
Merged

Conversation

Tahul
Copy link
Member

@Tahul Tahul commented Mar 9, 2025

❓ Type of change

  • πŸ‘Œ Enhancement (improving an existing functionality like performance)

πŸ“š Description

  • Drop CJS support (like upcoming version of vueuse)
  • Update to latest available vueuse version
  • Drop prettier (unused)

@Tahul Tahul requested a review from BobbieGoede March 9, 2025 23:45
@BobbieGoede BobbieGoede changed the title feat: vueuse maintenance update feat!: vueuse maintenance update Mar 10, 2025
@BobbieGoede
Copy link
Member

I just marked it as breaking with feat!, but not sure if it is.

@BobbieGoede
Copy link
Member

I think we can skip straight to vueuse v13 https://github.com/vueuse/vueuse/releases/tag/v13.0.0

Copy link

pkg-pr-new bot commented Mar 10, 2025

Open in Stackblitz

npm i https://pkg.pr.new/@vueuse/motion@254

commit: 22e7d2a

@BobbieGoede BobbieGoede changed the title feat!: vueuse maintenance update feat!: drop CJS and use VueUse v13 Mar 10, 2025
@BobbieGoede
Copy link
Member

@userquin
Could you take a look at the exports/types stuff whether things look right? πŸ™ I see you helped out with vueuse/vueuse#4581 too, you have the experience we lack πŸ˜…

@BobbieGoede BobbieGoede mentioned this pull request Mar 10, 2025
9 tasks
@userquin
Copy link
Contributor

ok, I'll check it later in the afternoon.

package.json Outdated
@@ -22,15 +22,12 @@
"sideEffects": false,
"exports": {
".": {
"import": "./dist/index.mjs",
"require": "./dist/index.cjs"
"import": "./dist/index.mjs"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

use ".": "./dist/index.mjs"

package.json Outdated
},
"./nuxt": {
"import": "./dist/nuxt/module.mjs",
"require": "./dist/nuxt/module.cjs"
"import": "./dist/nuxt/module.mjs"
Copy link
Contributor

Choose a reason for hiding this comment

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

use "./nuxt": "./dist/module.mjs"

}
},
"main": "./dist/index.cjs",
"module": "./dist/index.mjs",
"types": "./dist/index.d.ts",
"typesVersions": {
Copy link
Contributor

@userquin userquin Mar 10, 2025

Choose a reason for hiding this comment

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

since we're not using .js in the file extensions in the package, we need to add the d.mts for nuxt , "./*" isn't effective:

"typesVersions": {
  "*": {
    "nuxt": [
      "./dist/nuxt/module.d.mts"
    ]
  }
}

Copy link
Member

Choose a reason for hiding this comment

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

So should we have the following?

  "typesVersions": {
    "*": {
      "*": [
        "./dist/*",
        "./*"
      ],
      "nuxt": [
        "./dist/nuxt/module.d.mts"
      ]
    }
  },

Copy link
Contributor

Choose a reason for hiding this comment

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

or just use .js instead .mjs and keep current "typesVersions"

Copy link
Contributor

Choose a reason for hiding this comment

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

So should we have the following?

  "typesVersions": {
    "*": {
      "*": [
        "./dist/*",
        "./*"
      ],
      "nuxt": [
        "./dist/nuxt/module.d.mts"
      ]
    }
  },

no, just nuxt entry

Copy link
Contributor

Choose a reason for hiding this comment

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

in fact Nuxt will drop support for node10 module resolution (if not yet removed), we can remove "typesVersions" entry

Copy link
Contributor

Choose a reason for hiding this comment

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

Nuxt still supports node 10 , so it is fine

build.config.ts Outdated
@@ -2,7 +2,7 @@ import { defineBuildConfig } from 'unbuild'

export default defineBuildConfig({
rollup: {
emitCJS: true,
emitCJS: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

remove rollup entry, and change declaration to node16

package.json Outdated
}
},
"main": "./dist/index.cjs",
"module": "./dist/index.mjs",
"types": "./dist/index.d.ts",
Copy link
Contributor

Choose a reason for hiding this comment

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

use "types": "./dist/index.d.mts",

package.json Outdated
@@ -106,7 +103,6 @@
"lint-staged": "^15.2.5",
"nuxt": "^3.13.0",
"pkg-pr-new": "^0.0.20",
"prettier": "^3.2.5",
"typescript": "^5.4.5",
"unbuild": "^2.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

we should also update typescript and unbuild versions

},
"main": "./dist/index.cjs",
Copy link
Contributor

@userquin userquin Mar 10, 2025

Choose a reason for hiding this comment

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

add the main entry using "main": "./dist/index.mjs",

@userquin
Copy link
Contributor

userquin commented Mar 10, 2025

update tsconfig.json file removing vue-demi from paths, switch to "moduleResolution": "Bundler" and include these 2 changes:

"moduleDetection": "force",
"module": "preserve",

@userquin
Copy link
Contributor

userquin commented Mar 10, 2025

We should switch tests to use Vitest Browser Mode and test the library in the browser.

@BobbieGoede BobbieGoede merged commit d6dcf8f into main Mar 10, 2025
9 checks passed
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.

3 participants