Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Upgrade to @parse/push-adapter 6.4.0 #9182

Merged
merged 4 commits into from
Jul 8, 2024

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented Jul 7, 2024

Pull Request

Issue

The push adapter now supports ES6 and Node 22

*/node_modules/@parse/push-adapter/lib/WEB.js:59
var _ref = _asyncToGenerator( /#PURE/regeneratorRuntime.mark(function _callee(data, devices) {
^

ReferenceError: regeneratorRuntime is not defined
at /node_modules/@parse/push-adapter/lib/WEB.js:59:50
at /node_modules/@parse/push-adapter/lib/WEB.js:134:6
at Object. (/node_modules/@parse/push-adapter/lib/WEB.js:202:2)
at Module._compile (node:internal/modules/cjs/loader:1358:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1416:10)
at Module.load (node:internal/modules/cjs/loader:1208:32)
at Module._load (node:internal/modules/cjs/loader:1024:12)
at Module.require (node:internal/modules/cjs/loader:1233:19)
at require (node:internal/modules/helpers:179:18)
at Object. (/node_modules/@parse/push-adapter/lib/ParsePushAdapter.js:29:12)

Closes: parse-community/parse-server-push-adapter#256

Copy link

Thanks for opening this pull request!

@dplewis
Copy link
Member Author

dplewis commented Jul 8, 2024

@mtrezza The server tried to transcompile the ES module but is not supported. I add Babel and CommonJS support back to the push adapter.

parse-community/parse-server-push-adapter#266

@mtrezza
Copy link
Member

mtrezza commented Jul 8, 2024

Instead change the require of /home/runner/work/parse-server/parse-server/node_modules/@parse/push-adapter/src/index.js in /home/runner/work/parse-server/parse-server/lib/Controllers/index.js to a dynamic import() which is available in all CommonJS modules.

Can't we do this instead of reverting the push adapter repo to CommonJS? Maybe we support both modules and CommonJS with a try/catch where it require and if that fails as it does above uses dynamic import?

let module;
let modulePath;

try {
  module = require(modulePath);
} catch (err) {
  if (err.code === 'ERR_REQUIRE_ESM') {
    import(modulePath).then(mod => module = mod)
  } else {
    throw err;
  }
}

And we could write that as a method so we can re-use this in the future for other imports. Haven't tried this out though.

@dplewis
Copy link
Member Author

dplewis commented Jul 8, 2024

The trick here is /lib/Controllers/index.js. The lib folder is autogenerated by babel. I'm not sure how that can be done. To be honest this is my first time using ESM.

@mtrezza
Copy link
Member

mtrezza commented Jul 8, 2024

I've added some code above, not sure if you're seen it.

@dplewis
Copy link
Member Author

dplewis commented Jul 8, 2024

Oh thanks, let me try it out

@mtrezza
Copy link
Member

mtrezza commented Jul 8, 2024

It's an async import, as you have probably noticed. So this would need to go into an async function. If you paste it as-is it may be flaky, as the import sometimes may not have finished before the module is accessed.

@dplewis
Copy link
Member Author

dplewis commented Jul 8, 2024

I noticed and the Server doesnt' support top level await. The controllers are loaded in the ParseServer constructor so adding it to an async function is out of the question unless we do some rewriting.

@mtrezza
Copy link
Member

mtrezza commented Jul 8, 2024

Here's a better version:

async function loadModule(modulePath) {
  let module;

  try {
    module = require(modulePath);
  } catch (err) {
    if (err.code === 'ERR_REQUIRE_ESM') {
      module = await import(modulePath);
    } else {
      throw err;
    }
  }

  return module;
}

@dplewis
Copy link
Member Author

dplewis commented Jul 8, 2024

@mtrezza I keep running into Error: Adapter prototype don't match expected prototype. Can you see if you can get it to work?

Copy link

codecov bot commented Jul 8, 2024

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 94.15%. Comparing base (e7199e8) to head (b2df1cc).

Files Patch % Lines
src/Adapters/AdapterLoader.js 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #9182      +/-   ##
==========================================
- Coverage   94.16%   94.15%   -0.01%     
==========================================
  Files         186      186              
  Lines       14729    14738       +9     
==========================================
+ Hits        13869    13877       +8     
- Misses        860      861       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dplewis dplewis requested review from mtrezza and a team July 8, 2024 11:51
@dplewis
Copy link
Member Author

dplewis commented Jul 8, 2024

@mtrezza This is ready for review! I used 6.4.0 in production and it works.

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Wow, cool, I was trying around but got stuck in the async refactoring, glad you were able to fix this.

@mtrezza mtrezza merged commit ef1634b into parse-community:alpha Jul 8, 2024
24 of 26 checks passed
parseplatformorg pushed a commit that referenced this pull request Jul 8, 2024
# [7.1.0-alpha.15](7.1.0-alpha.14...7.1.0-alpha.15) (2024-07-08)

### Features

* Upgrade to @parse/push-adapter 6.4.0 ([#9182](#9182)) ([ef1634b](ef1634b))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.1.0-alpha.15

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Jul 8, 2024
parseplatformorg pushed a commit that referenced this pull request Jul 9, 2024
# [7.2.0-beta.1](7.1.0...7.2.0-beta.1) (2024-07-09)

### Bug Fixes

* Invalid push notification tokens are not cleaned up from database for FCM API v2 ([#9173](#9173)) ([284da09](284da09))

### Features

* Add support for dot notation on array fields of Parse Object ([#9115](#9115)) ([cf4c880](cf4c880))
* Upgrade to @parse/push-adapter 6.4.0 ([#9182](#9182)) ([ef1634b](ef1634b))
* Upgrade to Parse JS SDK 5.3.0 ([#9180](#9180)) ([dca187f](dca187f))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.2.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Jul 9, 2024
parseplatformorg pushed a commit that referenced this pull request Jul 9, 2024
# [7.2.0](7.1.0...7.2.0) (2024-07-09)

### Bug Fixes

* Invalid push notification tokens are not cleaned up from database for FCM API v2 ([#9173](#9173)) ([284da09](284da09))

### Features

* Add support for dot notation on array fields of Parse Object ([#9115](#9115)) ([cf4c880](cf4c880))
* Upgrade to @parse/push-adapter 6.4.0 ([#9182](#9182)) ([ef1634b](ef1634b))
* Upgrade to Parse JS SDK 5.3.0 ([#9180](#9180)) ([dca187f](dca187f))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 7.2.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Jul 9, 2024
@dplewis dplewis deleted the push-adapter branch July 16, 2024 18:48
@jaysonng
Copy link

Hi all. I just tried to update from parse-server 6.5.5 to 7.2.0 and got this error.

ReferenceError: regeneratorRuntime is not defined
    at /Users/jayson/Projects/Busog/parse-server/node_modules/parse-server-api-mail-adapter/lib/ApiMailAdapter.js:133:56
    at /Users/jayson/Projects/Busog/parse-server/node_modules/parse-server-api-mail-adapter/lib/ApiMailAdapter.js:166:6
    at Object.<anonymous> (/Users/jayson/Projects/Busog/parse-server/node_modules/parse-server-api-mail-adapter/lib/ApiMailAdapter.js:675:2)
    at Module._compile (node:internal/modules/cjs/loader:1256:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
    at Module.load (node:internal/modules/cjs/loader:1119:32)
    at Module._load (node:internal/modules/cjs/loader:960:12)
    at Module.require (node:internal/modules/cjs/loader:1143:19)
    at require (node:internal/modules/cjs/helpers:110:18)
    at Object.<anonymous> (/Users/jayson/Projects/Busog/parse-server/node_modules/parse-server-api-mail-adapter/lib/index.js:25:46)

Node.js v18.17.0

was this not supposed to happen at 7.2.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

regeneratorRuntime is not defined
4 participants