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) #27

Open
Phillip9587 opened this issue Oct 27, 2024 · 4 comments
Open

v2.0.0 - Major Release (Proposal) #27

Phillip9587 opened this issue Oct 27, 2024 · 4 comments

Comments

@Phillip9587
Copy link

I have created a PR (#26) that showcases these proposed changes.


Proposed Changes

  • Node Support:
    • Dropping support for Node.js versions that have been End-of-Life (EOL) for an extended period would be beneficial. While this is a critical topic, it would simplify our CI Pipeline and enable the use of modern syntax and tooling.
    • Focusing on supported Node versions would allow us to adopt and promote more modern features, moving the community forward.
    • Packages needing support for older Node versions could still transpile the code independently.
    • Alternative: Since this package does not rely on Node.js internals or specific dependencies, we could reduce the number of Node versions tested in CI. Officially, the package would be tested on key Node.js versions (e.g., v18 and above), with compatibility for older versions likely but not guaranteed.

New Features & Enhancements

  • ESM Support

    • Use Rollup to generate a CommonJS entry point (index.cjs) from an ESM entry point (index.mjs), as demonstrated in PR v2.0.0 - Major Release (Proposal) #26.
    • This approach improves compatibility with both modern and legacy build systems and enhances bundler and browser support.
    • It allows us to write modern ESM code and transpile it to CJS, bridging compatibility needs.
  • Built-in TypeScript Types

    • Embed TypeScript types directly within the package to eliminate the dependency on @types/content-type.
    • Since the types for this package are minimal, including them directly improves maintainability and reduces reliance on external packages.
    • If adopted, we should deprecate the @types/content-type package.

Maintainability Improvements

  • Updated CI Pipeline

    • Configure CI to test only supported Node.js versions (v18 and above) and potentially browsers if we decide to support them.
    • Replace nvm with the actions/setup-node action for better performance, leveraging its built-in caching for Node.js versions.
    • Limit CI pipeline permissions to essential scopes for security.
    • These changes mirror body-parser issue #545.
  • Native Testing with node:test

    • Replace mocha and nyc with Node.js’ native test runner and c8 for code coverage.
    • This reduces dependencies and streamlines the testing setup, aligning it with Node.js standards and keeping the environment lightweight.
  • ESLint Update

    • Upgrade to ESLint v9 and implement the new flat config to improve linting consistency.
  • Code Formatting

    • Consider introducing Prettier for consistent code style enforcement across the project.
    • Alternatively, we could add stylistic rules directly within the ESLint configuration, maintaining a single toolchain for both style and linting needs.

@bjohansebas
Copy link
Member

I think updating to ESLint 9 can wait, as we are opting to use standard or neostandard across the organization, although that's a discussion that has been going on for some time. @wesleytodd, has there been any progress on this?

@wesleytodd
Copy link
Member

Lots going on in this proposal, just like in the PR, which will make it hard to have a discussion around. I hate to ask again for changes in the proposal, but ideally this would be an issue per thing (or a PR per thing, depending on how much work you wanted to put into it with the chance of it being throw away). I have some concerns with the points above, but others could land without much worry.

@blakeembrey
Copy link
Member

Everything in the proposal looks great to me, except I'd skip ESM support for now. I think the dead code elimination for consumers and rollup in the dev pipeline does not outweigh the overhead for maintainers and overall package install size.

@blakeembrey
Copy link
Member

If we're already going to support types directly, why not use TypeScript and output both .js and .d.ts files so they're guaranteed to be in sync?

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

No branches or pull requests

4 participants