Skip to content

chore(ses): Advance Node.js compatibility test from version 10 to 12#1308

Merged
kriskowal merged 2 commits intomasterfrom
kris-goes-up-to-eleven
Oct 3, 2022
Merged

chore(ses): Advance Node.js compatibility test from version 10 to 12#1308
kriskowal merged 2 commits intomasterfrom
kris-goes-up-to-eleven

Conversation

@kriskowal
Copy link
Copy Markdown
Member

@kriskowal kriskowal commented Oct 1, 2022

Per #1306, we have an invalid Object.fromEntries shim that is only needed before Node.js 12 and analogous browsers. This change raises the tested lockdown compatibility floor up to Node.js 12.

  • fix(ses): Fix incompatible spelling
  • chore(ses): Advance lowest supported Node.js compatibility test from 10 to 12

Copy link
Copy Markdown
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@kriskowal kriskowal force-pushed the kris-goes-up-to-eleven branch from 44f9480 to 17c44e3 Compare October 1, 2022 01:33
@kriskowal kriskowal enabled auto-merge (rebase) October 1, 2022 01:33
@erights
Copy link
Copy Markdown
Contributor

erights commented Oct 2, 2022

ping. You've already enables auto-merge, but it seems stuck in CI.

pinging because #1306 waits for this one.

@turadg
Copy link
Copy Markdown
Member

turadg commented Oct 3, 2022

The problem was the branch protection rules were looking for the mispelling.
Screen Shot 2022-10-03 at 9 21 56 AM
I've updated to match the new correct spelling.

The branch wasn't up to date with master so I used GH UI to update with rebase.

@turadg turadg force-pushed the kris-goes-up-to-eleven branch from 17c44e3 to 92194e3 Compare October 3, 2022 16:23
@kriskowal kriskowal merged commit 6a4d3d5 into master Oct 3, 2022
@kriskowal kriskowal deleted the kris-goes-up-to-eleven branch October 3, 2022 16:29
erights added a commit that referenced this pull request Aug 27, 2024
Closes: #2418 
Refs: #2417 #1308 

## Description

Adapted from
#2419 (review)
below

The platform compatibility test specifically validates that SES works on
Node.js 12 and can be deleted since it has vanished into history.
Node.js 12 required special consideration because of its experimental
ESM support. Delete the `test:platform-compatibility` script in the ses
`package.json`, as well as the `test/package` fixture in ci.yml

Immediate motivation explained in #2418 , #2417 broken the
platform-compatibility-test tests because it depends on the platform
providing either `Array.prototype.transfer` or `structuredClone`. Node
supports `transfer` starting with Node 22, but supports
`structuredClone` starting in Node 18. That should be fine, since that
is our declared support floor. This PR changes our one known remaining
dependence on Node 12.

### Security Considerations

none
### Scaling Considerations

none
### Documentation Considerations

none beyond the need to explain our platform requirements, which this PR
does not change.
### Testing Considerations

The point. This PR only affects tests, not production code.

Reviewers, should this PR be labeled `test:` instead of `fix:`?

### Compatibility and Upgrade Considerations

After this PR, we will no longer notice further breakage on Node < 18.
That fits with our declared support floor.
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