ui5-community / ui5-community/babel-plugin-transform-modules-ui5

[BUG]: ExportCollapse undefined call expression

Open
#148 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
33
Forks
20
Avg merge
3h 22m
Merged PRs (30d)
1

Description

Hi,

I stumbled over the following issue which, while easy to resolve in many ways on the app dev side, I thought I might report - just in case.

It is triggered when all of the following are true:

  • The module has a default export and at least one named export.
  • The default export resolves to a value built from Object.assign(Object.create(null), { ... }).
  • Export collapsing is active (default behavior unless noExportCollapse is enabled).

The stack trace pointed to

  • node_modules/babel-plugin-transform-modules-ui5/dist/modules/helpers/exports.js:77

But could be traced back to actually being a result of:

export function getPropertiesOfObjectAssignOrExtendHelper(
  node,
  blockScopeNode
) {
  // Check all the args and recursively try to get props of identifiers (although they may be imported)
  return flatten(
    node.arguments.map((arg) => {
      if (t.isObjectExpression(arg)) {
        return arg.properties;
      }
      if (t.isIdentifier(arg)) {
        // Recursive, although props will be empty if arg is an imported object
        return getOtherPropertiesOfIdentifier(blockScopeNode, arg.name);
      }
      return [];     // possible fix
    })
  ).filter(Boolean); // possible fix
}

In here, the Object.create(null) of Object.assign(Object.create(null), { ... }) seems to add an undefined entry which later on fails in exports.js during:

if (!defaultExportProperty.key) {
  continue;
}

Quick Repro

const babel = require("@babel/core");
const plugin = require("babel-plugin-transform-modules-ui5");

const cases = [
  [
    "withAsConst",
    "const test=Object.assign(Object.create(null),{a:1}); export const TEST_CONSTANT='test' as const; export default test;",
  ],
  [
    "withoutAsConst",
    "const test=Object.assign(Object.create(null),{a:1}); export const TEST_CONSTANT='test'; export default test;",
  ],
  [
    "typeLiteralAnnotation",
    "const test=Object.assign(Object.create(null),{a:1}); export const TEST_CONSTANT:'test'='test'; export default test;",
  ],
  ["noNamedExport", "const test=Object.assign(Object.create(null),{a:1}); export default test;"],
  [
    "safeObjectAssign",
    "const test=Object.assign({}, {a:1}); export const TEST_CONSTANT='test' as const; export default test;",
  ],
  [
    "twoStepCreateAssign",
    "const test=Object.create(null); Object.assign(test,{a:1}); export const TEST_CONSTANT='test' as const; export default test;",
  ],
];

for (const [name, code] of cases) {
  try {
    babel.transformSync(code, {
      filename: `${name}.ts`,
      plugins: [plugin],
      parserOpts: { plugins: ["typescript"] },
    });
    console.log(`${name}: OK`);
  } catch (error) {
    console.log(`${name}: FAIL -> ${String(error.message).split("\n")[0]}`);
  }
}

One remark on contribution: I ran into several issues on windows in plenty of places in this repo, so I stopped bothering for now. I initially wanted to add a PR as well, as it is a seemingly small change/fix. But I also didn't want to overhaul half the repository just to get things working. I'm open though, maybe it was my mistake/error after all and maybe this isn't even a real issue that needs fixing. So an issue might even be the better way to go in the end. :)

BR

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the listed cases with Babel and inspect getPropertiesOfObjectAssignOrExtendHelper and exports.js:77 in babel-plugin-transform-modules-ui5. Compare the failing Object.assign(Object.create(null), ...) case with the safe and noNamedExport cases. Done means the failing cases transform without an undefined property error while existing behavior remains intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
babel, javascript, typescript
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.