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

docs: add note about line discrepancies and returning source maps in code transformers #12566

Merged
merged 3 commits into from
Apr 4, 2022

Conversation

mhnaeem
Copy link
Contributor

@mhnaeem mhnaeem commented Mar 10, 2022

Summary

Added a new note in the Code Transformation page about line discrepancies when source maps are not returned by the process or processAsync functions. Was just generally confusing why my errors were pointing to a different place.

Test plan

Only docs are changed -> proofread

Related Issue:

#10305 ?

@@ -128,6 +128,12 @@ As can be seen, only `process` or `processAsync` is mandatory to implement, alth

Note that [ECMAScript module](ECMAScriptModules.md) support is indicated by the passed in `supports*` options. Specifically `supportsDynamicImport: true` means the transformer can return `import()` expressions, which is supported by both ESM and CJS. If `supportsStaticESM: true` it means top level `import` statements are supported and the code will be interpreted as ESM and not CJS. See [Node's docs](https://nodejs.org/api/esm.html#esm_differences_between_es_modules_and_commonjs) for details on the differences.

:::info
Copy link
Member

Choose a reason for hiding this comment

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

Can we flip this around? maybe as a tip and something like "make sure your transformation returns a source map so it's possible to map back from the transformed code to the source code in code coverage and error information" or some such? Inline sourcemaps also work, doesn't have to be an explicit return value (although separate is preferred for performance reasons)

Copy link
Contributor Author

@mhnaeem mhnaeem Mar 11, 2022

Choose a reason for hiding this comment

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

Sure thing @SimenB. How does this sound?

Make sure TransformedSource contains a source map, so it is possible to report line information accurately in code coverage and test errors. Inline source maps also work but are slower.

I actually didn't know about inline source maps so when I was writing my own code transformer I had to separate our maps because I didn't know that option existed so having this kind of tip should be helpful.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

great, thanks! Could you copy it over to the versioned docs as well?

image

@github-actions
Copy link

github-actions bot commented May 5, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants