-
-
Notifications
You must be signed in to change notification settings - Fork 109
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this 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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
@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 :) |
const { pipeline } = require('node:stream/promises') | ||
import fastify from "fastify" | ||
import fs from "node:fs" | ||
import { pipeline } from "node:stream"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was /promises
removed intentionally?
Hello 👋
As the ESModules are the standard way to import/export modules in javascript, the README examples should reflect that :)