Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

rolandjitsu
Copy link
Collaborator

What kind of change does this PR introduce?

  • Bug Fix
  • Feature
  • Refactoring
  • Style
  • Build
  • Chore
  • Documentation
  • CI

Did you add tests for your changes?

  • Yes, my code is well tested
  • Not relevant

If relevant, did you update the documentation?

  • Yes, I've updated the documentation
  • Not relevant

Summary

Follow up to #1382. Attempt to fix the publint warnings:

➜  npx publint
react-dropzone lint results:
Suggestions:
1. The package does not specify the type field. NodeJS may attempt to detect the package type causing a small performance hit. Consider adding "type": "commonjs".
2. pkg.repository.url is https://github.com/react-dropzone/react-dropzone.git but could be a full git URL like "git+https://github.com/react-dropzone/react-dropzone.git".
3. The package publishes internal tests or config files. You can use pkg.files to only publish certain files and save user bandwidth.
Warnings:
1. pkg.exports["."].import.types types is interpreted as CJS when resolving with the "import" condition. This causes the types to be ambiguous when default importing the package due to its implied interop. Consider splitting out two "types" conditions for "import" and "require", and use the .mts extension, e.g. pkg.exports["."].import.import.types: "./typings/react-dropzone.d.mts"
2. pkg.exports["."].import.default is ./dist/es/index.js and is written in ESM, but is interpreted as CJS. Consider using the .mjs extension, e.g. ./dist/es/index.mjs

Does this PR introduce a breaking change?

It shouldn't, but I'm unable to test across browsers and different environments to tell for sure.

Other information

There were/are a few other changes/challenges while attempting to fix the warnings:

  1. Setting the type to module in package.json would most likely result in no support for commonjs, so instead, we used the mjs extension for the es distribution (which was one of the warnings)
  2. Tests would not longer work because of the above change, and after spending some time trying to fix it, what worked was switching to @swc/jest as a transform and a few other settings described in jest's ECMAScript Modules doc
  3. The other consequence was that eslint had to be updated too; so there are some changes around upgrading some of the packages and migrating to flat configs (this is WIP)
  4. We may not be able to go fwd with this if we stick to react-styleguidist; it currently does not build on the latest or current version; some additional work is required to see if we can make it work (webpack-blocks cannot be used, its deps are too old if we want to update webpack for react-styleguidist - some work around that started in https://github.com/react-dropzone/react-dropzone/tree/chore/update-deps)

@rolandjitsu rolandjitsu changed the title Fix/publint warnings fix: fix publint warnings Oct 15, 2024
@rolandjitsu rolandjitsu marked this pull request as draft October 15, 2024 16:53
@coveralls
Copy link

Pull Request Test Coverage Report for Build 835d81edb3fcdc458c49d210feaf8e85841e6f38-PR-1386

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.3%) to 98.485%

Totals Coverage Status
Change from base Build e707bda430c8c1d0acc881307c7cba7c1c01baa1: -0.3%
Covered Lines: 267
Relevant Lines: 269

💛 - Coveralls

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.