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: add entry to build info #1649

Closed
wants to merge 1 commit into from
Closed

Conversation

Hebilicious
Copy link
Contributor

@Hebilicious Hebilicious commented Aug 27, 2023

πŸ”— Linked issue

resolves #1650

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 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)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

While working on #1557, I noticed that for presets that can have multiple entries, like firebase, there's no straightforward way to identify which one has been used. This adds a reference to the preset used in the build info.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@@ -405,6 +405,7 @@ async function _build(nitro: Nitro, rollupConfig: RollupConfig) {
const buildInfo = {
date: new Date(),
preset: nitro.options.preset,
entry: nitro.options.entry.split("/").pop(),
Copy link
Member

@pi0 pi0 Aug 27, 2023

Choose a reason for hiding this comment

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

This is the source entry name not a build output information...

Copy link
Contributor Author

@Hebilicious Hebilicious Aug 27, 2023

Choose a reason for hiding this comment

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

Yes, this is useful for presets like firebase, where one preset can have multiple entries :
ie looking at the build output of a firebase preset build, we can't know if firebase-gen-1 or firebase-gen-2 is being used.

Copy link
Member

Choose a reason for hiding this comment

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

I know that's a valid case but generally it doesn't seems to be best idea to add source entry to production info. It can be implemented per preset imo.

Copy link
Contributor Author

@Hebilicious Hebilicious Aug 27, 2023

Choose a reason for hiding this comment

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

That's a good point, what about renaming the preset if it's firebase with a hook? Something like firebase_${variant} ?

Copy link
Member

Choose a reason for hiding this comment

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

Renaming the user-input preset can also lead to more confusing behavior.

@Hebilicious Hebilicious requested a review from pi0 August 27, 2023 19:11
@pi0
Copy link
Member

pi0 commented Sep 1, 2023

Thanks for the PR @Hebilicious. I guess i would prefer to use current method for firebase preset at least to keep the preset implementations simplicity and decoupled as much as possible.

While current approach for firebase gen handling is not perfect, i am happy to discuss about some other alternative ideas once we migrated repo to a monorepo. Also we can use virtual chunks for entry or simply support dynamic entry based on user input config (it was absence by the time of implementing that PR...)

@pi0 pi0 closed this Sep 1, 2023
@pi0 pi0 deleted the feat/build-info-entry branch October 28, 2023 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add entry information to build info
2 participants