Skip to content

Conversation

@andrewgremlich
Copy link
Contributor

@andrewgremlich andrewgremlich commented Oct 10, 2025

Integrating Vitest first test suite.

#17

@netlify
Copy link

netlify bot commented Oct 10, 2025

Deploy Preview for threejs-offset ready!

Name Link
🔨 Latest commit 42e9625
🔍 Latest deploy log https://app.netlify.com/projects/threejs-offset/deploys/68faa7a2ca235700095a5e3d
😎 Deploy Preview https://deploy-preview-22--threejs-offset.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@andrewgremlich
Copy link
Contributor Author

I've got a few things to check

@andrewgremlich
Copy link
Contributor Author

@AngyDev this PR is ready to go

@AngyDev
Copy link
Owner

AngyDev commented Oct 16, 2025

@andrewgremlich thank you, I'll review it in the weekend. Sorry at the moment I don't have the possibility to access to the code.

@AngyDev
Copy link
Owner

AngyDev commented Oct 19, 2025

Hi @andrewgremlich,

when I try to run the test or the server, I'm getting this warning

▲ [WARNING] The condition "types" here will never be used as it comes after both "import" and "require" [package.json]

    package.json:15:6:
      15 │       "types": "./lib/index.d.ts"
         ╵       ~~~~~~~

  The "import" condition comes earlier and will be used for all "import" statements:

    package.json:13:6:
      13 │       "import": "./lib/index.es.js",
         ╵       ~~~~~~~~

  The "require" condition comes earlier and will be used for all "require" calls:

    package.json:14:6:
      14 │       "require": "./lib/index.umd.js",

Do you have the same? If yes, could you fix it?

@andrewgremlich
Copy link
Contributor Author

Howdy @AngyDev ! Yup I'll take a look

@andrewgremlich
Copy link
Contributor Author

@AngyDev

Huh, for some reason the order matters in this section in package.json.

  "exports": {
    ".": {
      "import": "./lib/index.es.js",
      "require": "./lib/index.umd.js",
      "types": "./lib/index.d.ts"
    }
  },

Also, I realized that those exports don't point to the dist folder. Would you like that?

As well, I changed the nvmrc to point to the most recent node version (24).

@AngyDev
Copy link
Owner

AngyDev commented Oct 20, 2025

Yeah, the exports are only for the library. At this point, do we want to use version 25?

@andrewgremlich
Copy link
Contributor Author

We could use v25! I am initially against using it since it's not a designated LTS version. But since this isn't really an enterprise library, I guess it really doesn't matter. Thoughts?

@AngyDev
Copy link
Owner

AngyDev commented Oct 21, 2025

I was thinking the same. In this case, I don't think it is a big deal. Let's see what can happen 😅

@andrewgremlich
Copy link
Contributor Author

I was thinking the same. In this case, I don't think it is a big deal. Let's see what can happen 😅

As in let's go ahead with v25? I'm good with that

@andrewgremlich
Copy link
Contributor Author

@AngyDev this is ready to go I think

@AngyDev
Copy link
Owner

AngyDev commented Oct 29, 2025

@andrewgremlich Thanks for the updates! The new stuff looks good. Before we merge, can you revisit the older comments regarding the interfaces?

@andrewgremlich
Copy link
Contributor Author

@andrewgremlich Thanks for the updates! The new stuff looks good. Before we merge, can you revisit the older comments regarding the interfaces?

I'm so sorry... I don't see a comment about interfaces...

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.

2 participants