opencomponents / oc

OpenComponents, serverless in the front-end world for painless micro-frontends delivery
https://opencomponents.github.io/
MIT License
1.43k stars 122 forks source link

`Domain` library is deprecated! #989

Closed NimaSoroush closed 9 months ago

NimaSoroush commented 5 years ago

Who is the bug affecting?

Component creators

What is affected by this bug?

Node client

When does this occur?

in get-component.js > renderer() call

Where on the platform does it happen?

API

How do we replicate the issue?

Possibly by updating an OC component to use webpack 4

Expected behavior (i.e. solution)

The library should be replaced by a proper alternative or even better to rethink the current implementation

What version of OC, Node.js and OS are you using?

OC@v0.45.2 , Node v8.12.0, Mac

Other Comments

Deprecation note: https://nodejs.org/api/domain.html

error stack trace:

TypeError: processData is not a function at Domain.domain.run (/usr/local/abc/node_modules/oc/src/registry/routes/helpers/get-component.js:480:21) at Domain.run (domain.js:242:14) at repository.getDataProvider (/usr/local/abc/node_modules/oc/src/registry/routes/helpers/get-component.js:479:26) at cdn.getFile (/usr/local/abc/node_modules/oc/src/registry/domain/repository.js:247:9) at getFromAws (/usr/local/abc/node_modules/oc-s3-storage-adapter/index.js:125:7) at Response.getClient.getObject (/usr/local/abc/node_modules/oc-s3-storage-adapter/index.js:104:11) at Request.<anonymous> (/usr/local/abc/node_modules/aws-sdk/lib/request.js:364:18) at Request.callListeners (/usr/local/abc/node_modules/aws-sdk/lib/sequential_executor.js:106:20) at Request.emit (/usr/local/abc/node_modules/aws-sdk/lib/sequential_executor.js:78:10) at Request.emit (/usr/local/abc/node_modules/aws-sdk/lib/request.js:683:14) at Request.transition (/usr/local/abc/node_modules/aws-sdk/lib/request.js:22:10) at AcceptorStateMachine.runTo (/usr/local/abc/node_modules/aws-sdk/lib/state_machine.js:14:12) at /usr/local/abc/node_modules/aws-sdk/lib/state_machine.js:26:10 at Request.<anonymous> (/usr/local/abc/node_modules/aws-sdk/lib/request.js:38:9) at Request.<anonymous> (/usr/local/abc/node_modules/aws-sdk/lib/request.js:685:12) at Request.callListeners (/usr/local/abc/node_modules/aws-sdk/lib/sequential_executor.js:116:18) at Request.emit (/usr/local/abc/node_modules/aws-sdk/lib/sequential_executor.js:78:10) at Request.emit (/usr/local/abc/node_modules/aws-sdk/lib/request.js:683:14) at Request.transition (/usr/local/abc/node_modules/aws-sdk/lib/request.js:22:10) at AcceptorStateMachine.runTo (/usr/local/abc/node_modules/aws-sdk/lib/state_machine.js:14:12) at /usr/local/abc/node_modules/aws-sdk/lib/state_machine.js:26:10 at Request.<anonymous> (/usr/local/abc/node_modules/aws-sdk/lib/request.js:38:9)
matteofigus commented 5 years ago

Hi @NimaSoroush - as far as I remember, the domain library has been pending deprecation since node 6, but still on the pending state, while they finalize a replacement API: https://nodejs.org/dist/latest-v10.x/docs/api/domain.html I've been waiting for a long time for that, but still nothing new as far as I can tell.

This error looks interesting, as even in node 11 they mention the library is pending deprecation but yet should be there. For sure, if you are using node 8, that shouldn't be the case as I know of a number of people using node 8 in production with no issues.

Also, moving to webpack 4 shouldn't fix the issue as domain is actually central to how OC renders components both in the client and the server, so that couldn't help.

Can you double check your error stack trace is not coming from something else?

Thanks

NimaSoroush commented 5 years ago

Hi @matteofigus , webpack 4 is not the solution, is the possible root cause! Although we are still investigating.

Yeah, we are using node 8 and OC for most of our components without any issue. The component that throws this error is the only one adapted webpack 4.

The stack trace coming from OC internal process but that might be just a side effect. I will update this post once I have more info

NimaSoroush commented 5 years ago

Hi @matteofigus , After investigating different scenarios:

This is only happening in one of our old component. Trying all above combination in a fresh OC and all works fine!

So what is the difference?! After looking into opencomponents/oc code and seeing what would make the old component different from fresh OC I found this const processData = context.module.exports.data in https://github.com/opencomponents/oc/blob/b817be1e2f123dbde4265068aade58a0d3650d47/src/registry/routes/helpers/get-component.js#L475 This is generated by V8 Virtual Machine contexts. The old component exports default server in server.js which in plactice should be fine to be imported as data but for some reason when webpack bundles

webpack:4 and OC:0.45

...
/******/    // Load entry module and return exports
/******/    return __webpack_require__(__webpack_require__.s = "./src/server/server.js");
...
const server = (context, callback) => {
...
}
...
/* harmony default export */ __webpack_exports__["default"] = (server);

vs webpack:3 and OC:0.45

...
/******/    // Load entry module and return exports
/******/    return __webpack_require__(__webpack_require__.s = "./src/server/server.js");
...
const data = (context, callback) => {
...
}
...
/* harmony default export */ __webpack_exports__["default"] = (data);

So the quick resolution is to always export default data; in server.js of OCs. The proper fix would be to make neccessary changes to oc and make it to respect to default exports

ricardo-devis-agullo commented 9 months ago

Closing this since there's still no alternative in the horizon, and it's not a problem per se. As the node documentation states:

Users who absolutely must have the functionality that domains provide may rely on it for the time being but should expect to have to migrate to a different solution in the future.

We must have the functionality, so when a solution appears, we'll migrate.