fix: #893 #896

jerrywu001 · 2023-04-28T03:33:43Z

Fixes #893

src\index.ts (line 102)

+ // set tsconfig path from esbuildOptions function raw
+ if (!options.tsconfig && options.esbuildOptions) {
+   const regex = /(\w+)\.tsconfig\s*=\s*['"]([^'"]*)['"]/;
+   const match = options.esbuildOptions.toString().match(regex);
+   if (match) {
+     options.tsconfig = match[2];
+   }
+ }

const tsconfig = loadTsConfig(process.cwd(), options.tsconfig)

src\plugins\es5.ts (line 12)

    esbuildOptions(options) {
      if (options.target === 'es5') {
-       options.target = 'es2020'
+      // options.target = 'es2020'
        enabled = true
      }
    },

After the two, the test can pass (But it conflict with the other 2 test cases !!!!!!)

test('set target by esbuildOptions', async () => {
  await expect(
    run(
      getTestName(),
      {
        'input.ts': `const foo = 'foo';export default foo;`,
        'tsup.config.ts': `export default {
    esbuildOptions(options) {
      options.tsconfig = 'custom.tsconfig.json';
    },
  }`,
        'custom.tsconfig.json': `{
    "compilerOptions": {
      "target": "es5",
      "module": "esnext"
    }
  }`
      },
      { flags: [ '--format',  'cjs' ] }
    )
  ).rejects.toThrowError(
    `Transforming const to the configured target environment ("es5") is not supported yet`
  )
})

These got failed!!!!!! (line 849). because I deleted the options.target = 'es2020'

This is a problem. @egoist

test('es5 target', async () => {
  const { output, outFiles } = await run(
    getTestName(),
    {
      'input.ts': `
    export class Foo {
      hi (): void {
        let a = () => 'foo'

        console.log(a())
      }
    }
    `,
    },
    {
      flags: ['--target', 'es5'],
    }
  )
  expect(output).toMatch(/createClass/)
  expect(outFiles).toEqual(['input.js'])
})

codesandbox · 2023-04-28T03:33:45Z

Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

vercel · 2023-04-28T03:33:46Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
tsup	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 28, 2023 10:13am

jerrywu001 · 2023-04-28T03:39:48Z

There is a logical contradiction, see my specific description above. @egoist

ComLock · 2023-04-28T08:50:42Z

I may not understand the inner workings, but what I need to continue to work is:

https://tsup.egoist.dev/#es5-support

You can use --target es5 to compile the code down to es5, in this target your code will be transpiled by esbuild to es2020 first, and then transpiled to es5 by [SWC](https://swc.rs/).

So I suspect esbuild works with es2020 and then swc turns it back to es5 afterwards.

ComLock · 2023-04-28T08:53:52Z

When I set target: "es5" in the tsconfig.json I get an error, but when I set target: "es5" in the tsup.config.ts, it works as https://tsup.egoist.dev/#es5-support

So I suspect esbuild reads it's target from the tsconfig.json file.

ComLock · 2023-04-28T08:56:11Z

So I think this Pull Request will break ES5 support (https://tsup.egoist.dev/#es5-support)

jerrywu001 · 2023-04-28T08:58:13Z

So I think this Pull Request will break ES5 support (https://tsup.egoist.dev/#es5-support)

Of course, So, esbuild.build should set the target from --tsconfig path? @ComLock

Can you improve this pull request, and commit your update. @ComLock

ComLock · 2023-04-28T08:59:44Z

Ideally setting target only once, in one place would be nice. But the most important thing for me is the ES5 support (https://tsup.egoist.dev/#es5-support)

esbuild isn't able to do that yet, which is why swc is used.

ComLock · 2023-04-28T09:01:58Z

I can try, but it might be Greek to me :)

jerrywu001 · 2023-04-28T09:37:05Z

I can try, but it might be Greek to me :)

There seems to be no solution that is compatible with both scenarios.

ComLock · 2023-04-28T09:37:17Z

It seems esbuild looks into the tsconfig.json and throws errors before tsup looks into the tsconfig.custom.json

ComLock · 2023-04-28T10:09:08Z

So esbuild is probably used by bundle-require (in loadTsupConfig in load.ts) to transpile the tsup.config.ts file.

ComLock · 2023-04-28T10:18:32Z

So there are at least two "environments".

The environment tsup runs in (loads modules in)
The target environment

By default the tsconfig.json file is used by everyone, my code editor program too :)

It's possible to use a custom tsconfig.json file with bundle-require

jerrywu001 · 2023-04-28T10:21:40Z

So there are at least two "environments".

The environment tsup runs in (loads modules in)

The target environment

By default the tsconfig.json file is used by everyone, my code editor program too :)

It's possible to use a custom tsconfig.json file with bundle-require

So, I add an attribute to distinguish them.

I finished the job.

@ComLock @egoist

Tips:

--target=es5 and tsconfig.compilerOptions.target: "es5" can not use at the same time.
can only use --target es5 to compile the code down to es5.

ComLock · 2023-04-28T10:22:55Z

src/plugins/es5.ts

@@ -7,9 +7,8 @@ export const es5 = (): Plugin => {
  return {
    name: 'es5-target',

-    esbuildOptions(options) {
-      if (options.target === 'es5') {
-        options.target = 'es2020'


This still worries me. I think I will get an error from esbuild about it not supporting es5, rather than esbuild working with es2020 and then swc transpiling it back to es5?

I know what you worry about. See here, the usage:

--target es5/tsup.target=es5 and tsconfig.compilerOptions.target: "es5" can not use at the same time.

can only use --target es5/tsup.target=es5 to compile the code down to es5.

@ComLock

Just tested your branch and it breaks ES5 Support as I expected

It was a pleasure to work with you. @ComLock

Neither target on cmdline, in tsup.config.json nor tsconfig.custom.json works. All gives es5 not supported by esbuild errors.

all the best, after git pull, you should run npm run build, then run npm run test-only . @ComLock

In order to avoid the error I have to set esbuildOptions target: 'es2020' in addition to 'es5' (cmdline or tsup.config or tsconfig.custom)

no need to do this change @ComLock

it breaks the case

just this, it can pass all the cases

ComLock · 2023-04-28T10:30:29Z

src/plugins/es5.ts

@@ -7,9 +7,8 @@ export const es5 = (): Plugin => {
  return {
    name: 'es5-target',

-    esbuildOptions(options) {
-      if (options.target === 'es5') {
-        options.target = 'es2020'


Just tested your branch and it breaks ES5 Support as I expected

ComLock · 2023-04-28T10:55:00Z

Everything works without this patch, I closed the issue #893 because it was all confusion. See the closing comment on it.

ComLock · 2023-04-28T10:55:32Z

Good learning journey though :)

ComLock · 2023-04-28T11:05:40Z

Sidenote: egoist/bundle-require#38

fix: #893

fdeede4

vercel bot deployed to Preview April 28, 2023 03:33 View deployment

fix: update #893

003b496

vercel bot deployed to Preview April 28, 2023 10:13 View deployment

ComLock reviewed Apr 28, 2023

View reviewed changes

ComLock suggested changes Apr 28, 2023

View reviewed changes

jerrywu001 closed this Apr 28, 2023

Uh oh!

fix: #893 #896

fix: #893 #896

Uh oh!

Conversation

jerrywu001 commented Apr 28, 2023

Uh oh!

codesandbox bot commented Apr 28, 2023

Uh oh!

vercel bot commented Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jerrywu001 commented Apr 28, 2023

Uh oh!

ComLock commented Apr 28, 2023

Uh oh!

ComLock commented Apr 28, 2023

Uh oh!

ComLock commented Apr 28, 2023

Uh oh!

jerrywu001 commented Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ComLock commented Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ComLock commented Apr 28, 2023

Uh oh!

jerrywu001 commented Apr 28, 2023

Uh oh!

ComLock commented Apr 28, 2023

Uh oh!

ComLock commented Apr 28, 2023

Uh oh!

ComLock commented Apr 28, 2023

Uh oh!

jerrywu001 commented Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ComLock Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

jerrywu001 Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ComLock Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

jerrywu001 Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

ComLock Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

jerrywu001 Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ComLock Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

jerrywu001 Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ComLock Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

ComLock commented Apr 28, 2023

Uh oh!

ComLock commented Apr 28, 2023

Uh oh!

ComLock commented Apr 28, 2023

Uh oh!

Uh oh!

vercel bot commented Apr 28, 2023 •

edited

Loading

jerrywu001 commented Apr 28, 2023 •

edited

Loading

ComLock commented Apr 28, 2023 •

edited

Loading

jerrywu001 commented Apr 28, 2023 •

edited

Loading

jerrywu001 Apr 28, 2023 •

edited

Loading

jerrywu001 Apr 28, 2023 •

edited

Loading

jerrywu001 Apr 28, 2023 •

edited

Loading