The Wayback Machine - https://web.archive.org/web/20220409030741/https://github.com/loopbackio/loopback-next/issues/6393
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

Jump to bottom

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

Open
bajtos opened this issue Sep 18, 2020 · 6 comments
Open

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

bajtos opened this issue Sep 18, 2020 · 6 comments
Assignees
@itamar244
Labels
breaking-change feature good first issue Internal Tooling

Comments

@bajtos
Copy link
Member

@bajtos bajtos commented Sep 18, 2020

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.
@bajtos bajtos added feature good first issue Internal Tooling labels Sep 18, 2020
@bajtos bajtos changed the title Use --enable-source-maps instead of source-map-support on Node.js 12+ Use --enable-source-maps instead of source-map-support Sep 18, 2020
@itamar244
Copy link

@itamar244 itamar244 commented Oct 7, 2020

can I start work on this?
@itamar244
Copy link

@itamar244 itamar244 commented Oct 7, 2020

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

@dhmlau dhmlau commented Oct 10, 2020

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
Copy link
Member

@dhmlau dhmlau commented Oct 10, 2020

@itamar244, i just assigned it to you. Thanks.
@bajtos
Copy link
Member Author

@bajtos bajtos commented Oct 12, 2020

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
Copy link

@itamar244 itamar244 commented Oct 12, 2020

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 😄
@itamar244 itamar244 mentioned this issue Oct 13, 2020
Closed
8 tasks
@schickling schickling mentioned this issue Oct 22, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change feature good first issue Internal Tooling
3 participants
@bajtos @itamar244 @dhmlau