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

[Bug]: Polyfill:"Usage",but the packaged product es5 syntax has a scope problem, causing my variables to be contaminated. #8637

Open
PropersonCyber opened this issue Dec 6, 2024 · 15 comments
Assignees
Labels
bug Something isn't working

Comments

@PropersonCyber
Copy link

System Info

System:
OS: macOS 14.5
CPU: (8) x64 Apple M1
Memory: 30.48 MB / 16.00 GB
Shell: 5.9 - /bin/zsh
Binaries:
Node: 14.17.0 - ~/.nvm/versions/node/v14.17.0/bin/node
Yarn: 1.22.22 - /opt/homebrew/bin/yarn
npm: 6.14.13 - ~/.nvm/versions/node/v14.17.0/bin/npm
Browsers:
Chrome: 131.0.6778.87
Safari: 17.5

Details

I used polyfill:'usage' to configure compatibility with my browser, but the packaged product es5 syntax has a scope problem, causing my variables to be contaminated. In my code, show_type in 3 should be used instead of show_type in 1(figure 1 is the compiled code ). How do I fix this, Or is this a bug in rsbuild?

Image

Image

Reproduce link

https://codesandbox.io/p/github/rspack-contrib/rsbuild-codesandbox-example/csb-zktgdv/draft/great-flower?file=%2Fsrc%2FApp.jsx%3A11%2C54

Reproduce Steps

npm run build

checkout index.[hash].js , search "show_type" the result will be appear!!

@PropersonCyber PropersonCyber added bug Something isn't working pending triage The issue/PR is currently untouched. labels Dec 6, 2024
@AndyGuoX
Copy link

AndyGuoX commented Dec 6, 2024

Seems like I have the same problem

@chenjiahan
Copy link
Member

The codesandbox link is invalid, can you have a look at it?

Image

@PropersonCyber
Copy link
Author

The codesandbox link is invalid, can you have a look at it?

I checked. This connection is accessible. Would you please check if there is anything wrong with your network?

@PropersonCyber
Copy link
Author

The codesandbox link is invalid, can you have a look at it?

Or you can convert the following code with polyfill:usage

const composeIndicator = (keynode, logList) => { if (!keynode || keynode.length < 1) return []; for (let i in keynode) { const { key_list, relation, name, show_type } = keynode[i] || {}; if (!key_list || key_list.length < 1) continue; const pvArr = []; for (let j in key_list) { const { abbr, show_type, id, key_id } = key_list[j] || {}; const { pv, uv } = logList.find((i) => { if (id && i.id) { return i.id === id; } else if (key_id && i.key_id) { return i.key_id === key_id; } }) || {}; pvArr.push({ pv, uv, abbr, show_type }); } const dataList = []; for (let l in pvArr) { const { pv, abbr, show_type, uv } = pvArr[l] || {}; if (Array.isArray(pv)) { for (let k in pv) { if (!dataList[k]) { if (relation.indexOf(abbr) > -1) { let copyRelation = relation; const data = show_type === 1 ? pv[k] : uv[k]; const regx = new RegExp(${abbr}, 'g'); copyRelation = copyRelation.replace(regx, data); dataList.push(copyRelation); } } else { if (dataList[k].indexOf(abbr) > -1) { let copyRelation = dataList[k]; const data = show_type === 1 ? pv[k] : uv[k]; const regx = new RegExp(${abbr}, 'g'); copyRelation = copyRelation.replace(regx, data); dataList[k] = copyRelation; } } } } } keynode[i].pv = dataList.map((i) => { const calculate = eval(i); const val = show_type === 1 ? calculate === Infinity ? 0 : Math.round(calculate) : calculate === Infinity ? 0 : Number((calculate * 100).toFixed(2)); return isNaN(val) ? 0 : val; }); keynode[i].clicktag = name; } return keynode; };

@chenjiahan
Copy link
Member

You can provide a public GitHub repo instead.

@PropersonCyber
Copy link
Author

PropersonCyber commented Dec 9, 2024

You can provide a public GitHub repo instead.

Here, this is Github Repo: https://github.com/PropersonCyber/rebuild-demo.git
Run the command yarn build and check the index.[hash].js file. You should be able to reproduce the issue I'm experiencing.

@chenjiahan
Copy link
Member

Can you try adding output.minify: false to see if this is an issue of SWC minimizer?

@PropersonCyber
Copy link
Author

Can you try adding output.minify: false to see if this is an issue of SWC minimizer?

I've tried it before, and it does have problems, how can I avoid them when compatible with lower versions

Image

@chenjiahan
Copy link
Member

I think the current compilation result is correct:

Image

Which one do you think was not as expected?

@PropersonCyber
Copy link
Author

PropersonCyber commented Dec 9, 2024

I think the current compilation result is correct:

Image

Which one do you think was not as expected?

Yes, the relationship of variable correspondence you provided is correct. However, it appears there is an issue with variable contamination. The show_type variable indicated by the green arrow seems to be affected by the show_type variable pointed to by the red arrow. Could this be related to variable hoisting with var? I had assumed that instead of directly converting const and let to var, considerations would be made for scope. Interestingly, this issue doesn't occur when I use polyfill: 'none', as I'm utilizing let, which is restricted by block-level scope. So I think is there something wrong with the strategy.

Image

@chenjiahan
Copy link
Member

@GiveMe-A-Name Can you help to confirm if this is a bug in the SWC transformation? ❤️

@GiveMe-A-Name
Copy link
Member

GiveMe-A-Name commented Dec 11, 2024

I sure it's related to variable hoisting with var.
See a simple example:

var keynode = {
  show_type: 1,
  key_list: [
    { show_type: 'a', },
    { show_type: 'b' },
    { show_type: "c" },
  ]
}

var { show_type, key_list } = keynode;

console.log('show_type in loop output:', show_type);

for (var key in key_list) {
  var { show_type } = key_list[key];

  console.log("show_type in loop1:", show_type);

}

[1, 2, 3].forEach((i) => {
  console.log("show_type in loop2:", show_type);
})


// show_type in loop output: 1
// show_type in loop1: a
// show_type in loop1: b
// show_type in loop1: c
// show_type in loop2: c
// show_type in loop2: c
// show_type in loop2: c

It seems swc do not care about variable hoisting in this case.

The code would be transformed by babel, like bellow:

var composeIndicator = function composeIndicator(keynode, logList) {

  /// ...
  var dataList = [];
  for (var l in pvArr) {
    var _ref2 = pvArr[l] || {},
      pv = _ref2.pv,
      abbr = _ref2.abbr,
      // using a other name to avoid variable hoisting
      _show_type = _ref2.show_type,
      uv = _ref2.uv;
  }
  keynode[i].pv = dataList.map((i) => {

    var val = show_type === 1 ? calculate === Number.POSITIVE_INFINITY ? 0 : Math.round(calculate) : calculate === Number.POSITIVE_INFINITY ? 0 : Number((calculate * 100).toFixed(2));

  });


  /// ...
};

In short, it is a bug in swc

@PropersonCyber
Copy link
Author

I sure it's related to variable hoisting with var. See a simple example:

var keynode = {
show_type: 1,
key_list: [
{ show_type: 'a', },
{ show_type: 'b' },
{ show_type: "c" },
]
}

var { show_type, key_list } = keynode;

console.log('show_type in loop output:', show_type);

for (var key in key_list) {
var { show_type } = key_list[key];

console.log("show_type in loop1:", show_type);

}

[1, 2, 3].forEach((i) => {
console.log("show_type in loop2:", show_type);
})

// show_type in loop output: 1
// show_type in loop1: a
// show_type in loop1: b
// show_type in loop1: c
// show_type in loop2: c
// show_type in loop2: c
// show_type in loop2: c
It seems swc do not care about variable hoisting in this case.

The code would be transformed by babel, like bellow:

var composeIndicator = function composeIndicator(keynode, logList) {

/// ...
var dataList = [];
for (var l in pvArr) {
var _ref2 = pvArr[l] || {},
pv = _ref2.pv,
abbr = _ref2.abbr,
// using a other name to avoid variable hoisting
_show_type = _ref2.show_type,
uv = _ref2.uv;
}
keynode[i].pv = dataList.map((i) => {

var val = show_type === 1 ? calculate === Number.POSITIVE_INFINITY ? 0 : Math.round(calculate) : calculate === Number.POSITIVE_INFINITY ? 0 : Number((calculate * 100).toFixed(2));

});

/// ...
};
In short, it is a bug in swc

So this is related to the SWC used by Rspack, will it be fixed in the future?

@GiveMe-A-Name
Copy link
Member

GiveMe-A-Name commented Dec 11, 2024

So this is related to the SWC used by Rspack, will it be fixed in the future?

It was reported to swc. We will help swc to fix it.

@PropersonCyber
Copy link
Author

So this is related to the SWC used by Rspack, will it be fixed in the future?

It's report to swc. We will help swc to fix it.

Thank you for the clarification. I'm looking forward to the fix in future updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants