Skip to content

docs: Update README to use module imports #581

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AlvesJorge
Copy link

@AlvesJorge AlvesJorge commented May 19, 2025

Hello 👋
As the ESModules are the standard way to import/export modules in javascript, the README examples should reflect that :)

@AlvesJorge AlvesJorge changed the title update readme to use module imports Update readme to use module imports May 19, 2025
@AlvesJorge AlvesJorge changed the title Update readme to use module imports Update README to use module imports May 19, 2025
@AlvesJorge AlvesJorge changed the title Update README to use module imports docs: Update README to use module imports Jun 19, 2025
@mcollina mcollina requested a review from Fdawgs July 9, 2025 10:32
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@Fdawgs Fdawgs left a comment

Choose a reason for hiding this comment

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

Tentative on this, majority of our docs are in CJS and the underlying code is in CJS.

Likewise, CJS is still the default for Node in that it will treat all .js files as cjs unless the type is explicitly set to module in package.json.


fastify.register(require('@fastify/multipart'))
fastify.register(multipart)
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to await this

Copy link
Author

Choose a reason for hiding this comment

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

Why? I want to understand for my own personal use as well.

@AlvesJorge
Copy link
Author

Tentative on this, majority of our docs are in CJS and the underlying code is in CJS.

@Fdawgs would you not agree that Modules is the way node is headed and the majority of new code is written with modules? I understand that the underlying code is in CJS (and other parts of the docs too) but that wouldn't be a deterrent. If we want to move forward change needs to be incremental and non-breaking! We couldn't possibly update it all at once :)

@Fdawgs Fdawgs requested a review from a team August 14, 2025 15:45
const { pipeline } = require('node:stream/promises')
import fastify from "fastify"
import fs from "node:fs"
import { pipeline } from "node:stream";
Copy link
Contributor

Choose a reason for hiding this comment

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

@AlvesJorge

Was /promises removed intentionally?

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.

5 participants