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

feat: upgrade esbuild to 0.23 #615

Merged

Conversation

SuperchupuDev
Copy link
Contributor

@SuperchupuDev SuperchupuDev commented Jul 20, 2024

mainly to avoid duplicated dependencies, also bumped pkgroll so that it uses esbuild 0.23.0 as well

closes #606

@timrc
Copy link

timrc commented Aug 7, 2024

@SuperchupuDev Are there any major difference between esbuild versions? I mostly found code formatting changes.

@mesfahanisimscale
Copy link

evanw/esbuild#3802 It fixes these vulnerabilities.

@privatenumber
Copy link
Owner

How are those vulnerabilities exposed in tsx?

The response from the maintainer in that thread explains how that vulnerability report is a false alarm for esbuild.

(Am I missing something?)

@mesfahanisimscale
Copy link

mesfahanisimscale commented Aug 7, 2024

It may not be directly relevant, but many projects rely on automated vulnerability checks and it's a good practice to try to get those vulnerabilities down. But it's also understandable if tsx doesn't want to risk an upgrade for the sake of a number.

In our case, we've worked around it by adding this to our package.json.

  "overrides": {
    "tsx": {
      "esbuild": "0.23.0"
    }
  }

@privatenumber
Copy link
Owner

Sounds like an issue with your vulnerability checker. Please report the issue to them—there's a pattern of these security tools that leverage FUD to pressure unimpactful work on others.

And we're back to the first question: what practical impact does this upgrade have on tsx?

@yegortokmakov
Copy link

+1 to updating esbuild and comments in the thread. This specific vulnerability might be irrelevant to tsx, but handling CVE is a must for a lot of companies and a best practice in general. It is common to use something like https://github.com/openvex/vexctl to track identified vulnerabilities and either suppress them with description or track progress for fixes.

@SuperchupuDev
Copy link
Contributor Author

@privatenumber esbuild 0.22.0 comes with some ES2024 features such as the regular expression /v flag and it updates await using behavior to match typescript. is there any benefit not to update yet? as i don't think tsx will keep using an old esbuild version forever

@privatenumber
Copy link
Owner

Doesn't it raise any eyebrows for ya'll that no one can explain the practical impact of this work?

This is your time being wasted because of some security tool that profits from enterprise clients by blasting CVEs to create FUD.

Please don't involve my free open source projects into this.


@yegortokmakov
If you're telling me that you can suppress the vulnerability with an explanation, I'm confused why you opted to pressure a stranger online to do free work for you instead?

Would your company be open to an enterprise sponsorship? I'd be happy to do work for sponsors.

@SuperchupuDev
I appreciate your contribution. But as you know, tsx's goal is TypeScript parity.

I'm not against this PR, but I don't want to set the precedent of making releases without a reason. And it feels especially strange that no one can give a good reason.

Are you able to demonstrate how a TS feature currently broken in tsx can be fixed with this upgrade?

@mesfahanisimscale
Copy link

some security tool that profits from enterprise clients by blasting CVEs to create FUD.

Looks like a matter of principle for you. At the end it's your package and you do what you see fit.

@privatenumber
Copy link
Owner

It's more confusion that people are really trying to pressure strangers online to do free work to scratch their own itch without even scrutinizing the validity of the issue.

Anyway, to move forward, all I need is one demonstration of how a bug can be fixed with this change. Not much to ask for, I think. I will be hiding all comments that are not relevant to this.

@SuperchupuDev
Copy link
Contributor Author

@privatenumber here is repro code for something that's supposed to be fixed in esbuild 0.22.0, passes in typescript and fails in current tsx (see microsoft/TypeScript#58624, evanw/esbuild@6475aea)

image

code

export const output: any[] = [];
export async function main() {
  const promiseDispose = new Promise<void>(resolve => {
    setTimeout(() => {
      output.push('y dispose promise body');
      resolve();
    }, 0);
  });
  {
    await using x = {
      async [Symbol.asyncDispose]() {
        output.push('x asyncDispose body');
      }
    };
    await using y = {
      [Symbol.dispose]() {
        output.push('y dispose body');
        return promiseDispose;
      }
    };
  }
  output.push('body');
  await promiseDispose;
  return output;
}

export const a = async () => {
  const output2 = await main();
  const expected = ['y dispose body', 'x asyncDispose body', 'body', 'y dispose promise body'];
  if (output2.join(',') !== expected.join(',')) {
    throw `fail: ${output}`;
  }
  console.log('pass');
};

await a();

@SuperchupuDev
Copy link
Contributor Author

SuperchupuDev commented Aug 7, 2024

it works in latest esbuild:

image

@privatenumber
Copy link
Owner

Thanks @SuperchupuDev, I appreciate you demonstrating a real impact of this PR 🙌

Will look into releasing this tomorrow!

@privatenumber privatenumber changed the title ci: bump esbuild to 0.23.0 feat: upgrade esbuild to 0.23.0 Aug 8, 2024
@privatenumber privatenumber changed the title feat: upgrade esbuild to 0.23.0 fix: upgrade esbuild to 0.23 Aug 8, 2024
@privatenumber privatenumber changed the title fix: upgrade esbuild to 0.23 feat: upgrade esbuild to 0.23 Aug 8, 2024
@privatenumber privatenumber merged commit bd83d3b into privatenumber:master Aug 8, 2024
3 checks passed
@privatenumber
Copy link
Owner

This issue is now resolved in v4.17.0.

If you're able to, your sponsorship would be very much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support [email protected]
5 participants