Skip to content

Conversation

@lynnntropy
Copy link

Adds a plugin for Nitro.

@lynnntropy lynnntropy marked this pull request as ready for review December 23, 2025 12:25
Copy link
Member

@webpro webpro left a comment

Choose a reason for hiding this comment

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

Thanks, Lynn! Nice one, happy to merge. Added a few notes, but nothing major, I think.

@@ -0,0 +1,3 @@
{
"ignore": [".nitro/types/*.d.ts"]
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, users don't need to add this to their configuration.

We can add it to entry patterns in the plugin so the type definitions are added to the TS program.

Copy link
Author

Choose a reason for hiding this comment

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

Done.


const title = 'Nitro';

const enablers = ['nitropack'];
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

nitro is the new package for Nitro v3, nitropack is the package for v2 (the current stable release).

Now that you mention it, do you think it'd make sense for this plugin to support both v2 and v3, even though v3 isn't stable yet?

Copy link
Member

Choose a reason for hiding this comment

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

If you think the plugin implementation is largely the same for both the nitro and nitropack enablers, I think it makes sense to have just a single plugin

If they're quite different or if they should really have their own distinct title, then we should split it up I guess.

Copy link
Member

Choose a reason for hiding this comment

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

though v3 isn't stable yet?

From a Knip perspective this is likely stable enough?

}
},
"include": ["./nitro.d.ts", "../../**/*", "../../server/**/*"]
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we need all the TS configuration to cover the plugin's functionality properly? Path aliases, for instance, are covered elsewhere already. Please minimize to the essentials, if at all possible.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

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.

2 participants