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

add api to detect whether source-maps are enabled #46304

Closed
sapphi-red opened this issue Jan 22, 2023 · 6 comments · Fixed by #46391
Closed

add api to detect whether source-maps are enabled #46304

sapphi-red opened this issue Jan 22, 2023 · 6 comments · Fixed by #46391
Labels
feature request Issues that request new features to be added to Node.js. stale

Comments

@sapphi-red
Copy link
Contributor

What is the problem this feature will solve?

#39085 added process.setSourceMapsEnabled to enable source-maps programmatically. This is helpful for tools like Vite, thank you for implementing. 💚

But as this was implemented we now cannot know whether source-maps are enabled. I guess it was possible by process.execArgv.includes('--enable-source-maps') previously.

What is the feature you are proposing to solve the problem?

Add process.getSourceMapsEnabled that returns whether the source-maps are enabled.

What alternatives have you considered?

Add a getter + setter named process.isSourceMapsEnabled that works like process.getSourceMapsEnabled and process.setSourceMapsEnabled.

@sapphi-red sapphi-red added the feature request Issues that request new features to be added to Node.js. label Jan 22, 2023
@bnoordhuis
Copy link
Member

I read through the linked vite pull request but it wasn't clear to me why you need to know whether source maps are enabled. Can you explain? What's the use case?

@sapphi-red
Copy link
Contributor Author

Thanks for the response!

Vite exports a function that rewrites Error::stack to make the stack trace point to the original source file using source maps. When source maps are enabled on Node.js, Error::stack contains a stack trace that already points to the original source file. If the Error::stack already points to the original source file, that function should be no-op.

Because Vite nor the script using Vite knows whether the stack trace points to the original source file or the transformed file, we cannot make that function no-op.

// this is a pseudocode and won't work
import { load, rewriteStacktrace } from 'vite'

const mod = await load('./foo.js')

try {
  mod.error()
} catch (e) {
  console.log(e.stack) // this stack trace points to the transformed file
  rewriteStacktrace(e) // rewrite the stack trace
  console.log(e.stack) // this stack trace points to the original source file
}

@bnoordhuis
Copy link
Member

Right, that makes sense. getSourceMapsEnabled() already exists internally but would it work for you? It enables, as a side effect, source maps on first call when --enable-source-maps is set.

@sapphi-red
Copy link
Contributor Author

getSourceMapsEnabled() would work for me.

I know I should avoid using internal things, but is there any way to access the internal getSourceMapsEnabled()? If it's possible, I can support older versions.

@bnoordhuis
Copy link
Member

It's not exposed in any way (I think) but pull request welcome. I wouldn't object to a side-effect-free process.sourceMapsEnabled getter either; maybe that's even better.

sapphi-red added a commit to sapphi-red/node that referenced this issue Jan 28, 2023
Add `process.getSourceMapsEnabled` to detect
whether source-maps are enabled.

Fixes: nodejs#46304
@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Jul 25, 2023
sapphi-red added a commit to sapphi-red/node that referenced this issue Aug 15, 2023
Add `process.sourceMapsEnabled` to detect
whether source-maps are enabled.

Fixes: nodejs#46304
nodejs-github-bot pushed a commit that referenced this issue Aug 17, 2023
Add `process.sourceMapsEnabled` to detect
whether source-maps are enabled.

Fixes: #46304
PR-URL: #46391
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
UlisesGascon pushed a commit that referenced this issue Sep 10, 2023
Add `process.sourceMapsEnabled` to detect
whether source-maps are enabled.

Fixes: #46304
PR-URL: #46391
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
targos pushed a commit that referenced this issue Nov 15, 2023
Add `process.sourceMapsEnabled` to detect
whether source-maps are enabled.

Fixes: #46304
PR-URL: #46391
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
Add `process.sourceMapsEnabled` to detect
whether source-maps are enabled.

Fixes: nodejs/node#46304
PR-URL: nodejs/node#46391
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
Add `process.sourceMapsEnabled` to detect
whether source-maps are enabled.

Fixes: nodejs/node#46304
PR-URL: nodejs/node#46391
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
@avivkeller avivkeller moved this from Awaiting Triage to Done in Node.js feature requests Jun 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@bnoordhuis @sapphi-red and others