Skip to content

Conversation

socsieng
Copy link
Contributor

No description provided.

Copy link

pkg-pr-new bot commented Oct 15, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/@opennextjs/aws@556

commit: 4ee0641

Copy link
Contributor

@khuezy khuezy left a comment

Choose a reason for hiding this comment

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

Thanks so much for doing this! When ready, we should add this to the github workflow to run the unit test on PRs (synchronize)

test: "test",
},
multiValueQueryStringParameters: {
test: ["test"],
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when the same key (test) has different values? Do we want to merge the two? Is that even possible?
eg:

      queryStringParameters: {
        test: "test",
      },
      multiValueQueryStringParameters: {
        test: ["test-multi"],
      }

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it shouldn't be possible, hopefully aws has some kind of validation on this to avoid this kind of case.

@conico974
Copy link
Contributor

conico974 commented Oct 15, 2024

When ready, we should add this to the github workflow to run the unit test on PRs (synchronize)

It already runs on validate right now

@khuezy
Copy link
Contributor

khuezy commented Oct 15, 2024

Ah nice, I didn't realize.

Copy link
Contributor

@conico974 conico974 left a comment

Choose a reason for hiding this comment

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

That's very nice thank you.
LGTM

@conico974 conico974 merged commit 5486686 into opennextjs:main Oct 15, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants