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

feat: support async plugins #916

Closed
wants to merge 1 commit into from

Conversation

MarijnMensinga
Copy link

@MarijnMensinga MarijnMensinga commented Feb 6, 2023

πŸ”— Linked issue

#915

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Plugins should be loaded async to make the plugins bootstrap propperly.

This fixes potential race-conditions when creating plugins that are called before they are bootstrapped correctly.

Resolves #915

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly. (not needed)

@pi0 pi0 changed the title feat: make plugins load async feat: support async plugins Feb 7, 2023
@pi0
Copy link
Member

pi0 commented Feb 7, 2023

This is a nice feature to have! The main issue is that support for top-level await is limited and many providers are currently depending on nonasync API.

Putting a pending label for now. It is likely we could only support async plugins in Nitropack v3 and some breaking changes.

@Hebilicious
Copy link
Contributor

Hebilicious commented Jul 28, 2023

@pi0 Can't we work around the top level await issue with a closure ?

const createNitroApp = () => {

const installPlugins = async () => {
  for (const plugin of plugins) {
    await plugin(app);
  }
}

installPlugins()
}

@pi0
Copy link
Member

pi0 commented Jul 28, 2023

@Hebilicious no we lose order this way. (plugins can initialize too late and while/after first requests coming). Such example (dangling promises, can be already done inside a plugin and is a bad practice everywhere to left promises unanttended.

@pi0
Copy link
Member

pi0 commented Aug 6, 2023

Thanks for PR again dear @MarijnMensinga. It is not mergeable without breaking changes. Let's keep track via #915 (i might have some ideas to introduce as non breaking)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make plugins async
3 participants