fix(commonjs): handle global shorthand property correctly#1958
fix(commonjs): handle global shorthand property correctly#1958kaigritun wants to merge 1 commit intorollup:masterfrom
Conversation
When `global` is used as a shorthand property in an object literal (e.g.,
`{global}`), the plugin was replacing `global` with
`commonjsHelpers.commonjsGlobal`, resulting in invalid syntax
`{commonjsHelpers.commonjsGlobal}`.
This fix follows the same pattern used for the `require` shorthand case:
when detecting a shorthand property, prepend `global: ` to convert it to a
full property before replacing the value.
Fixes rollup/rollup#6242
|
This PR appears to duplicate #1957 |
|
@CharlieHelps please do a thorough review of this PR to determine if it's ready to merge. Look at the diff, reviews, comments, and referenced issues/docs/links to build a complete understanding. Then respond with a comment that includes a list of MUST fix items that must be fixed before the the PR is merged, and a list of IMPROVEMENTS that shouldn't block the merge but may be quick improvements or things that could be addressed in a follow up PR. The MUST fix items should explain exactly what's wrong and what needs to change to resolve the problem. Both should be numbered lists and the IMPROVEMENTS list should continue counting up from the MUST list. It's OK if there are no IMPROVEMENTS or MUST fix items, you can just omit the lists. |
|
Pulled this branch and ran MUST fix
IMPROVEMENTS |
Problem
When
globalis used as a shorthand property in an object literal (e.g.,{global}), the commonjs plugin replacesglobalwithcommonjsHelpers.commonjsGlobal, resulting in invalid syntax{commonjsHelpers.commonjsGlobal}.This causes a parse error like:
Root Cause
As identified in rollup/rollup#6242 by @TrickyPi, the CommonJS plugin transforms
{global}into{commonjsHelpers.commonjsGlobal}, which is invalid syntax because you can't have a dotted expression as a shorthand property.Solution
This fix follows the same pattern already used for the
requireshorthand case (see existingshorthand-requiretest): when detecting a shorthand property, prependglobal:to convert it to a full property before replacing the value.The transformation now correctly produces:
{global}→{commonjsHelpers.commonjsGlobal}(invalid syntax){global}→{global: commonjsHelpers.commonjsGlobal}(valid syntax)Changes
globalidentifier intransform-commonjs.jstest/fixtures/function/shorthand-global/Related Issues
Fixes rollup/rollup#6242