feat(evasive-transform)!: allow returns outside functions in CommonJS sources #2763
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm calling it breaking (and I've covered my bases) but others may disagree. |
kriskowal
approved these changes
Apr 9, 2025
naugtur
reviewed
Apr 9, 2025
| @@ -38,6 +36,7 @@ export function parseAst(source, opts = {}) { | |||
| return parseBabel(source, { | |||
| tokens: true, | |||
| createParenthesizedExpressions: true, | |||
| allowReturnOutsideFunction: opts.sourceType === 'script', | |||
There was a problem hiding this comment.
Suggested change
| allowReturnOutsideFunction: opts.sourceType === 'script', | |
| allowReturnOutsideFunction: opts.sourceType !== 'module', |
^ this would cover cases of sourceType not being set/known
There was a problem hiding this comment.
paraphrasing @kriskowal, it is only for script because we do not want to be toxic enablers of non-portable syntax
… sources BREAKING CHANGE: This restricts the possible values of the `sourceType` option to `script` and `module` only. Types have been changed to reflect this. *BREAKING*: Do not provide a `sourceType` of `unambiguous`; use `module`, `script`, or omit.
boneskull
force-pushed
the
boneskull/transform-top-lvl-return
branch
from
f6712c1 to
df96b6c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
returnstatements outside of functions are valid in CommonJS (sloppy-mode only? not sure), but Babel doesn't allow this by default. Unfortunately, people do this and We need to support it.This behavior is enabled by passing a flag into Babel when parsing. This flag is only enabled when the
sourceTypeoption (forevadeCensorandevadeCensorSycn) is set toscript.To discourage use of non-portable syntax, we now restrict the possible values of
sourceTypetoscriptandmoduleonly (which is the BREAKING CHANGE).The types were changed to reflect this. While updating the types, I noted that they were wrong, so I fixed them using overloads. Note that
@endo/evasive-transformdoes not ship declarations (yet).Security Considerations
No
Scaling Considerations
No
Documentation Considerations
I have updated
README.mdwith a little bit of information, but there are no such comprehensive API docs for this package.Testing Considerations
I've added tests to cover the new behavior.
Compatibility Considerations
If someone is using this package and supplying a
sourceTypeoption ofunambiguous(which I believe is Babel's default behavior), they will get a TypeScript error. We will not throw an exception, since this is a valid option for Babel, after all—but we do not claim to support it. This will not affect any existing consumers usingsourceTypeofscript, because if they were parsing sources that had barereturnstatements, it would have broken already.The new (correct) return types of the public APIs may cause TypeScript type errors if they were manually fixing the types themselves.
Upgrade Considerations
If using an explicit
sourceTypeofunambiguous, omit it. It is encouraged to usemoduleorscriptif possible.