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

fix: detect alt version of remix handler #1657

Merged
merged 4 commits into from
Nov 20, 2023
Merged

Conversation

Skn0tt
Copy link
Contributor

@Skn0tt Skn0tt commented Nov 17, 2023

While working on our Remix template, I found that the handler function that was generated for me isn't detected as v2, even though it should. This PR adds a test case for it.

It also adds the most straight-forward fix I found. It has the downside of creating a false-positive (will comment below) about that, but I think that's OK. Ideally though, we find a solution that doesn't require changes to the existing tests.

@Skn0tt Skn0tt self-assigned this Nov 17, 2023
@Skn0tt Skn0tt requested a review from a team as a code owner November 17, 2023 13:58
Copy link
Contributor

github-actions bot commented Nov 17, 2023

⏱ Benchmark results

Comparing with 5ca965f

largeDepsEsbuild: 1.4s

⬆️ 0.16% increase vs. 5ca965f

^   2.7s    2.7s    2.7s                                                                                  
│   ┌──┐    ┌──┐    ┌──┐                                    2.6s                                          
│   |  |    |  |    |  |                                    ┌──┐                                          
│   |  |    |  |    |  |                                    |  |                                          
│   |  |    |  |    |  |                                    |  |                                          
│   |  |    |  |    |  |                                    |  |                                          
│   |  |    |  |    |  |                                    |  |                                          
│ ──┼──┼────┼──┼────┼──┼────────────────────────────────────┼──┼──────────────────────────────────────────
│   |  |    |  |    |  |                                    |  |                                          
│   |  |    |  |    |  |    1.5s    1.5s    1.5s    1.5s    |  |    1.4s    1.5s    1.5s    1.4s    1.4s  
│   |  |    |  |    |  |    ┌──┐    ┌──┐    ┌──┐    ┌──┐    |  |    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    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

largeDepsNft: 5.6s

⬆️ 2.33% increase vs. 5ca965f

^   8.5s            8.5s                                                                                  
│   ┌──┐    8.2s    ┌──┐                                    8.1s                                          
│   |  |    ┌──┐    |  |                                    ┌──┐                                          
│   |  |    |  |    |  |                                    |  |                                          
│   |  |    |  |    |  |                                    |  |                                          
│   |  |    |  |    |  |                                    |  |                                          
│ ──┼──┼────┼──┼────┼──┼────────────────────────────────────┼──┼──────────────────────────────────────────
│   |  |    |  |    |  |    5.5s    5.6s    5.6s    5.7s    |  |    5.5s    5.6s    5.5s    5.5s    5.6s  
│   |  |    |  |    |  |    ┌──┐    ┌──┐    ┌──┐    ┌──┐    |  |    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    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

largeDepsZisi: 10.4s

⬇️ 0.32% decrease vs. 5ca965f

^  16.8s   16.5s   16.7s                                                                                  
│   ┌──┐    ┌──┐    ┌──┐                                   15.8s                                          
│   |  |    |  |    |  |                                    ┌──┐                                          
│   |  |    |  |    |  |                                    |  |                                          
│   |  |    |  |    |  |                                    |  |                                          
│   |  |    |  |    |  |                                    |  |                                          
│ ──┼──┼────┼──┼────┼──┼────────────────────────────────────┼──┼──────────────────────────────────────────
│   |  |    |  |    |  |   10.6s   10.6s   10.6s   10.5s    |  |   10.5s   10.5s   10.5s                  
│   |  |    |  |    |  |    ┌──┐    ┌──┐    ┌──┐    ┌──┐    |  |    ┌──┐    ┌──┐    ┌──┐   10.5s   10.4s  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐    ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    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

lukasholzer
lukasholzer previously approved these changes Nov 17, 2023
Copy link
Contributor

@lukasholzer lukasholzer left a comment

Choose a reason for hiding this comment

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

LGTM!

Test case taken from #1658.

Co-Authored-By: @lilnasy
eduardoboucas
eduardoboucas previously approved these changes Nov 20, 2023
tests/unit/runtimes/node/in_source_config.test.ts Outdated Show resolved Hide resolved
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
@Skn0tt Skn0tt merged commit 46c96e2 into main Nov 20, 2023
10 checks passed
@Skn0tt Skn0tt deleted the v2-detect-remix-handler-alt branch November 20, 2023 12:55
Skn0tt added a commit to netlify/build that referenced this pull request May 21, 2024
…1657)

* fix: detect alt version of remix handler

* chore: add another test for astro-style config

Test case taken from netlify/zip-it-and-ship-it#1658.

Co-Authored-By: @lilnasy

* Update tests/unit/runtimes/node/in_source_config.test.ts

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

---------

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants