feat: include resolved included_files in response of `zipFunction` #1098

danez · 2022-05-30T17:34:41Z

Summary

Return the resolved paths of included_files from the config. This will allow the cli to watch these files and rebuild the function on demand.

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

github-actions · 2022-05-30T17:56:48Z

⏱ Benchmark results

Comparing with 0d22eac

largeDepsEsbuild: 7.6s

⬆️ 14.11% increase vs. 0d22eac

^   8.1s                                                                                                  
│   ┌──┐    7.8s                                                                                    7.6s  
│   |  |    ┌──┐            7.2s                                                                    ┌──┐  
│   |  |    |  |            ┌──┐    6.8s                                            6.8s            |▒▒|  
│ ──┼──┼────┼──┼────6.3s────┼──┼────┌──┐────6.5s────6.5s────6.5s────────────────────┌──┐────6.6s────|▒▒|──
│   |  |    |  |    ┌──┐    |  |    |  |    ┌──┐    ┌──┐    ┌──┐    6.2s    6.3s    |  |    ┌──┐    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐    ┌──┐    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T

Legend

T-30 (0d22eac): 6.5s
T-29 (846798a): 7.7s
T-28 (4335698): 8.2s
T-27 (7e95f8a): 6.1s
T-26 (4ae024e): 6.3s
T-25 (e329986): 6.3s
T-24 (aad5de4): 6.1s
T-23 (6419b22): 8.3s
T-22 (f178b2c): 6.4s
T-21 (eecd8ed): 7.7s
T-20 (28a85fb): 6.3s
T-19 (61e5b72): 6.6s
T-18 (43ab287): 6.1s
T-17 (1e5cd4a): 6.7s
T-16 (5378020): 6.4s
T-15 (87e11b4): 9.3s
T-14 (55679c9): 6.5s
T-13 (d2d5f28): 6.4s
T-12 (68765f6): 8.1s
T-11 (4ac07ab): 7.8s
T-10 (3287a69): 6.3s
T-9 (4cad17d): 7.2s
T-8 (b0da17e): 6.8s
T-7 (b4a14d8): 6.5s
T-6 (1ffe7c1): 6.5s
T-5 (e5d1c7f): 6.5s
T-4 (400cf3c): 6.2s
T-3 (b510094): 6.3s
T-2 (dc0fe1a): 6.8s
T-1 (ad9161d): 6.6s
T (current commit): 7.6s

largeDepsNft: 36.1s

⬆️ 15.80% increase vs. 0d22eac

^  37.5s    37s                                                                                           
│   ┌──┐    ┌──┐                                                                                   36.1s  
│   |  |    |  |                                                                                    ┌──┐  
│   |  |    |  |            32s    31.7s                   31.4s                                    |▒▒|  
│ ──┼──┼────┼──┼───30.5s────┌──┐────┌──┐───30.1s───29.6s────┌──┐───────────29.6s────30s────29.4s────|▒▒|──
│   |  |    |  |    ┌──┐    |  |    |  |    ┌──┐    ┌──┐    |  |   28.9s    ┌──┐    ┌──┐    ┌──┐    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T

Legend

T-30 (0d22eac): 30.4s
T-29 (846798a): 35.9s
T-28 (4335698): 39.1s
T-27 (7e95f8a): 27.2s
T-26 (4ae024e): 30.5s
T-25 (e329986): 28s
T-24 (aad5de4): 28.1s
T-23 (6419b22): 39.9s
T-22 (f178b2c): 28s
T-21 (eecd8ed): 40.5s
T-20 (28a85fb): 29.1s
T-19 (61e5b72): 30.7s
T-18 (43ab287): 27.6s
T-17 (1e5cd4a): 32s
T-16 (5378020): 30.5s
T-15 (87e11b4): 44.6s
T-14 (55679c9): 30.2s
T-13 (d2d5f28): 31s
T-12 (68765f6): 37.5s
T-11 (4ac07ab): 37s
T-10 (3287a69): 30.5s
T-9 (4cad17d): 32s
T-8 (b0da17e): 31.7s
T-7 (b4a14d8): 30.1s
T-6 (1ffe7c1): 29.6s
T-5 (e5d1c7f): 31.4s
T-4 (400cf3c): 28.9s
T-3 (b510094): 29.6s
T-2 (dc0fe1a): 30s
T-1 (ad9161d): 29.4s
T (current commit): 36.1s

tests/main.js

src/runtimes/node/index.ts

tests/main.js

eduardoboucas

Great work! I wonder if there's anything we could/should do to make the terminology clearer, because we have srcFiles (which represents files identified by the bundler) and includedFiles (which represents files identified by the user), and that might not be obvious by the naming alone?

What do you think? Can we rename something to make it clearer? Or maybe some comments to help guide people in the right direction?

eduardoboucas · 2022-06-02T14:12:58Z

Oh, I should've clarified that my questions above are non-blocking. I think it's fine to ship this as is, just wondering if there's anything we should do in a follow up issue/PR.

danez · 2022-06-02T14:56:07Z

src/runtimes/node/bundlers/esbuild/src_files.ts

-    srcFiles: [...includedPaths, mainFile],
-    includedFiles: filterExcludedPaths(includedFilePaths, excludedPaths),
+    srcFiles: [...srcFiles, ...includedPaths, mainFile],
+    includedFiles: includedPaths,


I just made some minor adjustments to every bundler, so that we do not iterate over includedFilePaths twice.

Nice! That also makes things a bit clearer, IMO.

danez · 2022-06-02T14:58:34Z

Great work! I wonder if there's anything we could/should do to make the terminology clearer, because we have srcFiles (which represents files identified by the bundler) and includedFiles (which represents files identified by the user), and that might not be obvious by the naming alone?

What do you think? Can we rename something to make it clearer? Or maybe some comments to help guide people in the right direction?

Yeah it had me confused as well. There is also input which contains all files that are the input to the bundling, including resolved dependencies.
Also afaics every bundler slightly differs in what is included in srcFiles. I think we should definitely come back to this.

eduardoboucas · 2022-06-02T15:10:36Z

I think we should definitely come back to this

Do you mind creating an issue so that is captured somewhere?

danez · 2022-06-02T15:34:50Z

Created #1099

…etlify/zip-it-and-ship-it#1098) * feat: include resolved included_files in response of `zipFunction` * fix: do not iterate multiple times over includedFiles

danez added type: feature code contributing to the implementation of a feature and/or user facing functionality area: functions labels May 30, 2022

danez force-pushed the included_files-resolved branch from 892e58e to 55429b0 Compare May 30, 2022 17:43

danez marked this pull request as ready for review May 30, 2022 17:47

danez requested a review from a team May 30, 2022 17:47

danez mentioned this pull request May 30, 2022

fix: correctly watch function files specified in the config option included_files netlify/cli#4619

Merged

danez force-pushed the included_files-resolved branch from 55429b0 to 7efc193 Compare May 30, 2022 17:54

eduardoboucas reviewed May 31, 2022

View reviewed changes

tests/main.js Outdated Show resolved Hide resolved

src/runtimes/node/index.ts Outdated Show resolved Hide resolved

eduardoboucas reviewed Jun 2, 2022

View reviewed changes

tests/main.js Outdated Show resolved Hide resolved

feat: include resolved included_files in response of zipFunction

3dbebdd

danez force-pushed the included_files-resolved branch from 7efc193 to 3dbebdd Compare June 2, 2022 13:55

eduardoboucas previously approved these changes Jun 2, 2022

View reviewed changes

fix: do not iterate multiple times over includedFiles

2e1becf

danez dismissed eduardoboucas’s stale review via 2e1becf June 2, 2022 14:54

danez requested a review from eduardoboucas June 2, 2022 14:55

danez commented Jun 2, 2022

View reviewed changes

eduardoboucas approved these changes Jun 2, 2022

View reviewed changes

danez mentioned this pull request Jun 2, 2022

Clarify some return values around paths and files of zipFunction #1099

Open

danez added the automerge label Jun 2, 2022

kodiakhq bot merged commit 830f9f2 into main Jun 2, 2022

kodiakhq bot deleted the included_files-resolved branch June 2, 2022 15:35

token-generator-app bot mentioned this pull request Jun 2, 2022

chore(main): release 5.10.0 #1092

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: include resolved included_files in response of `zipFunction` #1098

feat: include resolved included_files in response of `zipFunction` #1098

danez commented May 30, 2022 •

edited

Loading

github-actions bot commented May 30, 2022

eduardoboucas left a comment

eduardoboucas commented Jun 2, 2022

danez Jun 2, 2022 •

edited

Loading

eduardoboucas Jun 2, 2022

danez commented Jun 2, 2022 •

edited

Loading

eduardoboucas commented Jun 2, 2022

danez commented Jun 2, 2022

feat: include resolved included_files in response of zipFunction #1098

feat: include resolved included_files in response of zipFunction #1098

Conversation

danez commented May 30, 2022 • edited Loading

Summary

github-actions bot commented May 30, 2022

⏱ Benchmark results

largeDepsEsbuild: 7.6s

largeDepsNft: 36.1s

eduardoboucas left a comment

Choose a reason for hiding this comment

eduardoboucas commented Jun 2, 2022

danez Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

eduardoboucas Jun 2, 2022

Choose a reason for hiding this comment

danez commented Jun 2, 2022 • edited Loading

eduardoboucas commented Jun 2, 2022

danez commented Jun 2, 2022

feat: include resolved included_files in response of `zipFunction` #1098

feat: include resolved included_files in response of `zipFunction` #1098

danez commented May 30, 2022 •

edited

Loading

danez Jun 2, 2022 •

edited

Loading

danez commented Jun 2, 2022 •

edited

Loading