search

loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.94k stars 1.06k forks source link

Use `--enable-source-maps` instead of `source-map-support` #6393

Open bajtos opened 4 years ago

bajtos commented 4 years ago

Starting from version 12.12.0, Node.js offers built-in support for translating error stack traces using source maps - learn more here: https://medium.com/@nodejs/source-maps-in-node-js-482872b56116

Let's rework @loopback/build to use this new feature on recent version of Node.js, instead of source-map-support module.

If it's possible, then implement a fallback to npm module source-map-support when running on Node.js 10.x.

If that's not possible, then this change must wait until we can drop support for Node.js 10.x, which will happen after 10.x reaches EOL on 2021-04-30 per https://github.com/nodejs/Release.

itamar244 commented 3 years ago

can I start work on this?

itamar244 commented 3 years ago

should I remove it entirely from the loopback repo, or only when internally testing @loopback/build?

dhmlau commented 3 years ago

can I start work on this?

Absolutely!

should I remove it entirely from the loopback repo, or only when internally testing @loopback/build?

IIUC, the change would be limited to @loopback/build.

dhmlau commented 3 years ago

@itamar244, i just assigned it to you. Thanks.

bajtos commented 3 years ago

My idea is to modify lb-mocha and/or the default .mocharc.js file to leverage Node.js built-in --enable-source-maps option instead of depending on a 3rd-party module source-map-support. As Diana wrote, the change will be limited to @loopback/build, but since all LB4 code is using @loopback/build to run the tests, this change will affect other code too.

Here is the place where we are configuring source-map-support:

https://github.com/strongloop/loopback-next/blob/8f932797f66c27213bf4812860d087ebebf7d467/packages/build/config/.mocharc.json#L2

We must carefully consider impact on existing consumers of @loopback/build. I think the safest option is to publish this change as semver-major, see Making breaking changes in our dev docs.

itamar244 commented 3 years ago

I added the --enable-source-maps to the mocha file conditionally depending on the Node version, using semver package and .mocharc.js file for custom logic instead of .json file. it seems to work.

I will submit a working PR tommorow 😄