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

v2.0.0 - Major Release (Proposal) #26

Closed
wants to merge 7 commits into from
Closed

Conversation

Phillip9587
Copy link

This pull request proposes a new major version (v2.0.0) of the content-type package. This is only a proposal and was opened to start the discussion.

Breaking Changes:

  • Set minimum required Node.js version to v18:
    • The minimum supported Node.js version is now v18. This decision was made to align with the minimum version required by the express package.
    • The minimum version could technically be lowered to any Node.js version supporting ES2015.

New Features & Enhancements:

  • Dual Package Support:

    • Rollup is used to generate the CommonJS Entrypoint (index.cjs) from the ESM Entrypoint (index.mjs).
    • This enhances compatibility with both modern and legacy build systems and adds better bundler and browser support.
  • Updated CI Pipeline:

    • CI is now configured to test only against supported Node.js versions (v18 and above).
    • The actions/setup-node action is used instead of nvm because it caches Node.js versions on the action runners.
  • Native Testing with Node.js:

    • Replaced mocha and nyc with Node.js’ native test runner and c8 for code coverage. This reduces dependencies and simplifies the testing setup, making it more lightweight and aligned with Node.js standards.
  • ESLint Configuration Update:

    • Improved linting setup with ESLint v9 and the new flat config.
  • Script for Ensuring CJS File Consistency:

    • Added a script that checks if the index.cjs file is up to date with the ESM build. This ensures both module formats stay consistent.
    • The script is run in the CI Pipeline so that checks fail if the builds are inconsistent.

To Be Discussed:

  • Should TypeScript types be added directly to the package?:

    • Instead of relying on the @types/content-type package, TypeScript types could be added directly to the package.
    • If we proceed with this, the @types/content-type needs to be deprecated.
  • Should the husky package be used for pre-push hooks?:

    • The husky package could be used to add a pre-push hook that runs the check-build script to ensure the build is consistent before pushing changes.
  • Should a formatter like Prettier be used?:

    • Should we introduce a code formatter like Prettier to enforce a consistent code style across the project?
    • Alternatively, we could add ESLint stylistic rules to handle code style enforcement directly within the linting process, avoiding the need for a separate formatter.

Again, this is just a proposal and up for discussion. No hard feelings if the changes are not accepted!

@Phillip9587 Phillip9587 marked this pull request as ready for review October 23, 2024 11:19
@bjohansebas
Copy link
Member

bjohansebas commented Oct 23, 2024

Hi @Phillip9587, I like the idea of aligning with Express, and I understand that this is a proposal, maybe for the future if you want to continue doing this in other packages, you can open an issue, just like it was done in compression and response-time, so that the captains of each package are informed and it aligns with the future plans they have for their package

@UlisesGascon
Copy link
Member

UlisesGascon commented Oct 23, 2024

I agree with @bjohansebas. Also making huge PRs is not ideal, as they can be blocked for a while due to multiple conversations on different topics. This also makes it harder for others to review. 👍

Convert this into an issue will be the way to go @Phillip9587 :)

@blakeembrey
Copy link
Member

@Phillip9587 I do love this PR but I think we need to take it in a couple of steps, just because there's some other changes I also want to make to the package that's likely to conflict. I'm not sure about shipping both CJS and MJS, or bumping the minimum node right now unless we're also making other breaking changes.

Should TypeScript types be added directly to the package?

I would like to see this.

Should the husky package be used for pre-push hooks?
Should a formatter like Prettier be used?

I'm in favor of both, but prefer to avoid adding too many config files or churning on tooling.

@whawker
Copy link

whawker commented Oct 24, 2024

Would be great to have ESM support for this library, but I totally appreciate that is easy for me to say as a non maintainer! 😄

@Phillip9587
Copy link
Author

@bjohansebas @UlisesGascon @blakeembrey Thanks for your feedback. I wrote a proposal: #27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants