Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

feat: Make additional function attributes available via functions manifest #1221

Merged
merged 26 commits into from
Jan 12, 2023

Conversation

jackiewmacharia
Copy link
Contributor

@jackiewmacharia jackiewmacharia commented Oct 14, 2022

πŸŽ‰ Thanks for submitting a pull request! πŸŽ‰

Summary

Issue: https://github.com/netlify/pod-compute/issues/192
Issue: https://github.com/netlify/pod-compute/issues/156
Makes displayName, nodeBundler and isInternalFunction available to the API


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code πŸ§‘β€πŸ’».
    This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing
    a typo or something that`s on fire πŸ”₯ (e.g. incident related), you can skip this step.
  • Read the contribution guidelines πŸ“–. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) πŸ§ͺ
  • Update or add documentation (if features were changed or added) πŸ“
  • Make sure the status checks below are successful βœ…

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

@jackiewmacharia jackiewmacharia changed the title Make additional function attributes available via functions manifest (feat): Make additional function attributes available via functions manifest Oct 14, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 14, 2022

⏱ Benchmark results

Comparing with de48712

largeDepsEsbuild: 3s

⬆️ 32.68% increase vs. de48712

^                                                    3s                      3s   
β”‚                                                   β”Œβ”€β”€β”                    β”Œβ”€β”€β”  
β”‚                                                   |  |    2.8s            |β–’β–’|  
β”‚           2.6s                                    |  |    β”Œβ”€β”€β”            |β–’β–’|  
β”‚           β”Œβ”€β”€β”                                    |  |    |  |            |β–’β–’|  
β”‚ ──────────┼──┼────2.2s────────────────────────────┼──┼────┼──┼────────────|β–’β–’|──
β”‚           |  |    β”Œβ”€β”€β”    2.1s    2.1s    2.1s    |  |    |  |            |β–’β–’|  
β”‚    2s     |  |    |  |    β”Œβ”€β”€β”    β”Œβ”€β”€β”    β”Œβ”€β”€β”    |  |    |  |     2s     |β–’β–’|  
β”‚   β”Œβ”€β”€β”    |  |    |  |    |  |    |  |    |  |    |  |    |  |    β”Œβ”€β”€β”    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

largeDepsNft: 11.9s

⬆️ 36.19% increase vs. de48712

^          13.4s                                                                  
β”‚           β”Œβ”€β”€β”                                                                  
β”‚           |  |                                   11.8s                   11.9s  
β”‚           |  |                                    β”Œβ”€β”€β”                    β”Œβ”€β”€β”  
β”‚           |  |   10.6s                            |  |   10.9s            |β–’β–’|  
β”‚           |  |    β”Œβ”€β”€β”   10.3s                    |  |    β”Œβ”€β”€β”            |β–’β–’|  
β”‚ ──9.4sβ”€β”€β”€β”€β”Όβ”€β”€β”Όβ”€β”€β”€β”€β”Όβ”€β”€β”Όβ”€β”€β”€β”€β”Œβ”€β”€β”β”€β”€β”€β”€9.7s────────────┼──┼────┼──┼────────────|β–’β–’|──
β”‚   β”Œβ”€β”€β”    |  |    |  |    |  |    β”Œβ”€β”€β”            |  |    |  |            |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |     8s     |  |    |  |            |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    β”Œβ”€β”€β”    |  |    |  |    7.6s    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    β”Œβ”€β”€β”    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

largeDepsZisi: 21.6s

⬆️ 30.92% increase vs. de48712

^                                                  21.2s                   21.6s  
β”‚                                                   β”Œβ”€β”€β”   20.7s            β”Œβ”€β”€β”  
β”‚                                                   |  |    β”Œβ”€β”€β”            |β–’β–’|  
β”‚          18.9s                                    |  |    |  |            |β–’β–’|  
β”‚           β”Œβ”€β”€β”   16.8s                            |  |    |  |            |β–’β–’|  
β”‚ β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”Όβ”€β”€β”€β”€β”Œβ”€β”€β”β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€15.8s────┼──┼────┼──┼────────────|β–’β–’|──
β”‚           |  |    |  |   15.6s   15.3s    β”Œβ”€β”€β”    |  |    |  |   14.9s    |β–’β–’|  
β”‚  14.4s    |  |    |  |    β”Œβ”€β”€β”    β”Œβ”€β”€β”    |  |    |  |    |  |    β”Œβ”€β”€β”    |β–’β–’|  
β”‚   β”Œβ”€β”€β”    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
β”‚   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |β–’β–’|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

@jackiewmacharia jackiewmacharia changed the title (feat): Make additional function attributes available via functions manifest feat: Make additional function attributes available via functions manifest Oct 14, 2022
@github-actions github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Oct 14, 2022
@khendrikse khendrikse self-assigned this Jan 9, 2023
@khendrikse khendrikse force-pushed the feat/pass-additional-function-attributes branch from c656390 to f5993ec Compare January 9, 2023 15:22
@khendrikse khendrikse marked this pull request as ready for review January 10, 2023 13:47
@khendrikse khendrikse requested a review from a team January 10, 2023 13:47
@khendrikse khendrikse marked this pull request as draft January 10, 2023 13:47
@khendrikse khendrikse marked this pull request as ready for review January 10, 2023 13:49
danez
danez previously requested changes Jan 10, 2023
src/config.ts Outdated Show resolved Hide resolved
src/runtimes/node/bundlers/esbuild/bundler_target.ts Outdated Show resolved Hide resolved
src/utils/check_is_internal_function.ts Outdated Show resolved Hide resolved
tests/main.test.ts Outdated Show resolved Hide resolved
@khendrikse khendrikse force-pushed the feat/pass-additional-function-attributes branch 3 times, most recently from 731567f to dd92b53 Compare January 11, 2023 10:10
@khendrikse khendrikse marked this pull request as draft January 11, 2023 12:28
@khendrikse khendrikse force-pushed the feat/pass-additional-function-attributes branch from 763f07d to 9f5b99b Compare January 11, 2023 14:38
@khendrikse khendrikse marked this pull request as ready for review January 11, 2023 15:27
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tests/zip_function.test.ts Outdated Show resolved Hide resolved
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.

I left some comments that I think we should address (the path normalisation changes), but other than that it's looking great! ✨

README.md Outdated Show resolved Hide resolved
src/runtimes/node/bundlers/zisi/resolve.ts Outdated Show resolved Hide resolved
src/runtimes/node/utils/module.ts Outdated Show resolved Hide resolved
tests/main.test.ts Outdated Show resolved Hide resolved
tests/main.test.ts Outdated Show resolved Hide resolved
tests/main.test.ts Outdated Show resolved Hide resolved
tests/main.test.ts Outdated Show resolved Hide resolved
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.

πŸš€

@khendrikse khendrikse dismissed danez’s stale review January 12, 2023 11:05

changed a lot!

@khendrikse khendrikse merged commit ea4008e into main Jan 12, 2023
@khendrikse khendrikse deleted the feat/pass-additional-function-attributes branch January 12, 2023 11:05
Skn0tt pushed a commit to netlify/build that referenced this pull request May 21, 2024
…ifest (netlify/zip-it-and-ship-it#1221)

* feat: pass additional-function-attributes

* chore: make attributes available to functions manifest

* fix: fix linting

* fix: use optional chaining

* fix: add ext

* fix: fix tests

* fix: change internal directory name in constants.ts

* feat: add test for go functions with displayName and from an internal functions folder

* feat: add test for rust functions build from internal folder with displayName property

* feat: add test for nodejs functions build from internal folder with displayName property

* test: figure out windows breakn

* fix: take windows paths into account when checking for internal functions

* test: add unit test for checkIsInternalFunction

* docs: add new return values

* test: move tests into a better fixtures structure

* fix: use unixify for checkIsInternalFunction, config, bundler, module and resolve

* feat: add internalFunctionsFolder option to zipfunctionoptions

* test: fix tests

* test: add test for zipFunction and code to support that as well

* test: change toBeTruthy to toBe(true) and isInternalFunction to internalFunction

* chore: change displayName input to name

* chore: change internalFunction to isInternal

* chore: revert unixify, rename tests and internalFunctionsFolder to internalSrcFolder

* test: revert unixify for module to see if windows test still fails

* Revert "test: revert unixify for module to see if windows test still fails"

This reverts commit b444d1c4828965d72d909d96fa68e531a888d221.

Co-authored-by: khen <30577427+khendrikse@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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

4 participants