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

docs: Remove ES module with NodeNext note #5361

Merged
merged 1 commit into from Mar 19, 2024
Merged

Conversation

melroy89
Copy link
Contributor

@melroy89 melroy89 commented Mar 15, 2024

#4241 and smartiniOnGitHub/fastify-healthcheck#20 both seems to be closed. This note can be removed.

Checklist

fastify#4241 seems to closed.

Signed-off-by: Melroy van den Berg <melroy@melroy.org>
@melroy89 melroy89 changed the title Remove ES module warning Remove ES module note Mar 15, 2024
@melroy89 melroy89 changed the title Remove ES module note Remove ES module with NodeNext note Mar 15, 2024
@jsumners jsumners requested a review from a team March 16, 2024 13:50
@climba03003
Copy link
Member

The issue of tracking the problem is
#4349

@mcollina mcollina changed the title Remove ES module with NodeNext note docs: Remove ES module with NodeNext note Mar 18, 2024
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

or @climba03003 are you blocking this?

I think this works now?

@climba03003
Copy link
Member

I think this works now?

For plugins, Yes. All of them should be migrated and works now.

Are you blocking this?

Not blocking

@melroy89
Copy link
Contributor Author

melroy89 commented Mar 18, 2024

For plugins, Yes. All of them should be migrated and works now.

Only a slight issue with the auto load plugin. But that just needs to be fixed ASAP. And this comment is unrelated and not the answer anyway. For auto load plugin I need to set VITEST environment variable when using ESM in combination with Ts-Node.

See fastify/fastify-autoload#309 (comment) and my original help request: fastify/help#1014 (comment)

@mcollina mcollina merged commit 1a4972c into fastify:main Mar 19, 2024
5 of 10 checks passed
@mcollina
Copy link
Member

A PR to autoload would be amazing

@melroy89
Copy link
Contributor Author

melroy89 commented Mar 19, 2024

A PR to autoload would be amazing

If I knew how to solve ESM detection yes. Sorry I'm not too familiar with autoload and how to detect ESM.

But yes, it's the only plugin that cause issues! So it's the last fastify plugin that needs (re)work for proper ESM support.

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.

None yet

3 participants