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

Regression in 0.18.2: decorators should prevent tree-shaking #3164

Closed
njfamirm opened this issue Jun 15, 2023 · 7 comments
Closed

Regression in 0.18.2: decorators should prevent tree-shaking #3164

njfamirm opened this issue Jun 15, 2023 · 7 comments

Comments

@njfamirm
Copy link

hi everyone
yesterday i'm update esbuild to to latest version.
I'm tried to correctly upgrade to 0.18.x based release note, and don't find my problem.

Thank you for telling me my problem according to my configs

esbuild.js

const esbuildContext = await esbuild.context({
  entryPoints: [`${srcDir}/${srcFilename}.ts`],

  logLevel: 'info',
  platform: 'browser',
  target: 'es2018',
  format: 'esm',
  conditions: debugMode ? ['development'] : undefined,

  minify: !prettyMode,
  treeShaking: true,
  sourcemap: true,
  sourcesContent: debugMode,
  bundle: true,
  splitting: true,
  charset: 'utf8',
  legalComments: 'none',
  metafile: true,

  define: {
    _ALWATR_VERSION_: `'${packageJson.version}+${gitShortSha}'`,
  },
  // drop: ['debugger'],

  loader: {
    '.png': 'copy',
    '.jpg': 'copy',
    '.woff': 'copy',
    '.woff2': 'copy',
  },

  banner: {
    js: banner,
    css: banner,
  },

  outbase: srcDir,
  outdir: outDir,
  assetNames: 'asset/[name]-[hash]',
  entryNames: watchMode ? '[name]' : '[dir]/[name]-[hash]',
  chunkNames: 'chunks/[name]-[hash]',
});

const esBuildPromise = esbuildContext.rebuild();

tsconfig.base.json

{
  "compilerOptions": {
    "incremental": true,
    "target": "ES2019",
    "module": "NodeNext",
    "moduleResolution": "NodeNext",
    "resolveJsonModule": true, // temporary for fix import json issue
    "lib": ["ES2022", "DOM", "DOM.Iterable"],
    "declaration": true,
    "declarationMap": true,
    "outDir": "./",
    "composite": true,
    "tsBuildInfoFile": ".tsbuildinfo",
    "removeComments": false,
    "importHelpers": true,
    "isolatedModules": true, // need for esbuild
    "strict": true,
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictBindCallApply": true,
    "strictPropertyInitialization": true,
    "noImplicitThis": true,
    "alwaysStrict": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "noImplicitOverride": true,
    "allowSyntheticDefaultImports": true,
    "sourceMap": true,
    "inlineSources": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "skipLibCheck": true,
    "forceConsistentCasingInFileNames": true,
    "useDefineForClassFields": false,
    "listEmittedFiles": true
  }
}

tsconfig.json

{
  "extends": "../../tsconfig.base",
  "compilerOptions": {
    "composite": true,
    "tsBuildInfoFile": ".tsbuildinfo",
    // "moduleResolution": "bundler",
    "rootDir": "src",
    "outDir": "build",
    "noEmit": true,
    "plugins": [
      {
        "name": "ts-lit-plugin",
        "strict": true
      }
    ],
  }}

Thanks to all the contributors of this good project!

@njfamirm njfamirm changed the title BUG: not bundle all files with new esbuild not bundle all files with new esbuild Jun 15, 2023
@evanw
Copy link
Owner

evanw commented Jun 15, 2023

I'm marking this issue as unactionable because it does not have a self-contained way to reproduce the issue and/or an adequate description of the problem. There's nothing I can do with this issue report as it currently is. This issue will be later closed if the necessary information is not provided.

@njfamirm
Copy link
Author

I thought you might be able to find the problem by looking at tsconfig.json
It does not show the bug that I want to post here
The typescript files are nothing special and are the same as before
As I said, some of the imports are not included in the bundle code
If they were in the previous version

But anyway, this typescript file

@evanw
Copy link
Owner

evanw commented Jun 16, 2023

You still have not posted a self contained way to reproduce the issue and/or an adequate description of the problem. I’m not going to hunt through your code to try to figure out what your issue is.

@AliMD
Copy link

AliMD commented Jun 16, 2023

We looooose more than 30h time for debugging on same issue 😭
Its about #3161
@clydin Thank you so much.
@evanw Could you please .....

@dmitrage
Copy link
Contributor

Maybe the reason is that esbuild considers decorated classes to be side-effect free since 0.18.2. Got caught by this while trying to update deps in an app where components are registered using decorators.

0.18.1
https://esbuild.github.io/try/#YgAwLjE4LjEALS1idW5kbGUAZQBlbnRyeS50cwBAU0lERV9FRkZFQ1QgCmNsYXNzIFRlc3Qge30AAHRzY29uZmlnLmpzb24AewogICJjb21waWxlck9wdGlvbnMiOiB7CiAgICAiZXhwZXJpbWVudGFsRGVjb3JhdG9ycyI6IHRydWUKICB9Cn0

0.18.2
https://esbuild.github.io/try/#YgAwLjE4LjIALS1idW5kbGUAZQBlbnRyeS50cwBAU0lERV9FRkZFQ1QgCmNsYXNzIFRlc3Qge30AAHRzY29uZmlnLmpzb24AewogICJjb21waWxlck9wdGlvbnMiOiB7CiAgICAiZXhwZXJpbWVudGFsRGVjb3JhdG9ycyI6IHRydWUKICB9Cn0

Was this (breaking?) change expected? Should the app codebase be migrated?

@evanw
Copy link
Owner

evanw commented Jun 20, 2023

Thanks for the report. That's really a new issue, but since this issue isn't actionable and I still don't know what it was about, I can make this issue about your problem instead. It's not intentional that decorators are now considered side-effect free. I'll fix that in the next release.

@evanw evanw changed the title not bundle all files with new esbuild Regression in 0.18.2: decorators should prevent tree-shaking Jun 20, 2023
@evanw evanw closed this as completed in d565272 Jun 20, 2023
@AliMD
Copy link

AliMD commented Jun 21, 2023

I love you so much for finally...
All projects written with github.com/lit/lit have encountered this problem and gone to hell like mine! 😢

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

No branches or pull requests

4 participants