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

fix(build): prevent destructure with reuse of same symbol. #6504

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Dec 31, 2024

This is the cause of the UMD e2e failure in next.

Basically the flow is:

  1. source code that destructures something in arguments:
function fn({ one, two }) {}
  1. esm output of source code
function fn(_ref) {
  let {
    one,
    two,
  } = _ref
  1. umd production output (using uglify)
function fn(e) {
  var { one: e, two: t } = e

note the reused variable name

  1. output after import through parcel of the prod umd import
function fn(e) {
  var e = e.one,
    t = e.two;

This fixes it by changing step 3 by using terser to:

function fn(e) {
  let { one: t, two: n } = e;
}

note that it doesn't reuse the e variable and thus no longer bugs and has this output in parcel:

function fn(e) {
  var t = e.one,
    n = e.two;
}

This issue started showing up once we changed the browser output of the js umd build to keep destructuring, which had this "rename-bug". Updating to terser is the right solution for the future as it's actually kept updated unlike uglify.

This is the cause of the UMD e2e failure in `next`.

Basically the flow is:

1. source code that destructures something in arguments:

```js
function fn({ one, two }) {}
```

2. esm output of source code

```js
function fn(_ref) {
  let {
    one,
    two,
  } = _ref
```

3. umd production output (using uglify)

```js
function fn(e) {
  var { one: e, two: t } = e
```

note the reused variable name

4. output after import through parcel of the prod umd import

```js
function fn(e) {
  var e = e.one,
    t = e.two;
```

This fixes it by changing step 3 by using terser to:

```js
function fn(e) {
  let { one: t, two: n } = e;
}
```

note that it doesn't reuse the `e` variable and thus no longer bugs and has this output in parcel:

```js
function fn(e) {
  var t = e.one,
    n = e.two;
}
```

This issue started showing up once we changed the browser output of the js umd build to keep destructuring, which had this "rename-bug". Updating to terser is the right solution for the future as it's actually kept updated unlike uglify.
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2f9f119:

Sandbox Source
example-instantsearch-getting-started Configuration
example-react-instantsearch-getting-started Configuration
example-react-instantsearch-next-app-dir-example Configuration
example-react-instantsearch-next-routing-example Configuration
example-vue-instantsearch-getting-started Configuration

Copy link
Member

@dhayab dhayab left a comment

Choose a reason for hiding this comment

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

🤘

@Haroenv Haroenv merged commit d00cc09 into next Dec 31, 2024
11 checks passed
@Haroenv Haroenv deleted the fix/prevent-destructure-mistake branch December 31, 2024 15:10
Haroenv added a commit that referenced this pull request Jan 9, 2025
This is the cause of the UMD e2e failure in `next`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants