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: return generated manifest object #48

Merged
merged 6 commits into from
Jun 21, 2022

Conversation

EwanValentine
Copy link
Contributor

🎉 Thanks for sending this pull request! 🎉

Please make sure the title is clear and descriptive.

If you are fixing a typo or documentation, please skip these instructions.

Otherwise please fill in the sections below.

  • UPDATED: When the edge manifest is generated, this PR returns the generated manifest object, which means we don't have to load the manifest from the generated file in order to debug/inspect the output in other packages where needed.

Please add a x inside each checkbox:

  • I have read the contribution guidelines.
  • The status checks are successful (continuous integration). Those can be seen below.

A picture of a cute animal (not mandatory but encouraged)

carlos-capuchin-1024x1024

@EwanValentine EwanValentine requested a review from a team June 14, 2022 14:53
@github-actions github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Jun 14, 2022
@eduardoboucas
Copy link
Member

  • When the edge manifest is generated, this PR returns the generated manifest object, which means we don't have to load the manifest from the generated file in order to debug/inspect the output in other packages where needed.

I would argue that actually reading the file from its expected location at deploy time would be part of the diagnostic. Any reason you prefer to expose it programmatically and skip that step?

@EwanValentine
Copy link
Contributor Author

EwanValentine commented Jun 14, 2022

Hmmm, I figured the contents of the manifest were the important part, rather than the loading of the manifest file itself. So figured it would be cleaner to just return it, rather than reload the contents from the file again. I wasn't 100% sure on that though, so happy to close this and just reload it in the builder

@eduardoboucas
Copy link
Member

Hmmm, I figured the contents of the manifest were the important part, rather than the loading of the manifest file itself. So figured it would be cleaner to just return it, rather than reload the contents from the file again. I wasn't 100% sure on that though, so happy to close this and just reload it in the builder

No objections from me! I just wanted to offer an alternative option in case you hadn't considered it, as it could avoid making changes to Edge Bundler.

src/bundler.ts Outdated Show resolved Hide resolved
@EwanValentine EwanValentine self-assigned this Jun 21, 2022
Copy link
Member

@eduardoboucas eduardoboucas left a comment

Choose a reason for hiding this comment

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

Nice! ✨

@EwanValentine EwanValentine merged commit a9eadcf into main Jun 21, 2022
@EwanValentine EwanValentine deleted the feat/return-generated-manifest-object branch June 21, 2022 16:09
Skn0tt pushed a commit to netlify/build that referenced this pull request Apr 23, 2024
* feat: return generated manifest object

* chore: updated options params
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants