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

Exception in production mode of webpack #38

Closed
HaydenOrz opened this issue Feb 20, 2024 · 13 comments
Closed

Exception in production mode of webpack #38

HaydenOrz opened this issue Feb 20, 2024 · 13 comments

Comments

@HaydenOrz
Copy link
Contributor

HaydenOrz commented Feb 20, 2024

Description

In Webpack's production mode, if the optimization.minimize option is turned on (Webpack has this option turned on by default), will get an error.

image

image

It looks like it might be an issue with the compression tool, but I feel the need to report this issue to you, because webpack is very widely used.

Minimal demo to reproducing the problem

https://github.com/HaydenOrz/antlr4ng-trino


The good news is that Antlr4ng performs well in all of these situations:

  • NodeJS
  • Vite
  • Webpack development mode
  • webpack production mode and turn the optimization.minimize option off
@mike-lischke
Copy link
Owner

mike-lischke commented Feb 20, 2024

Usually, the error that a type is not defined when running a TS/JS app, means that it wasn't imported yet (because of circular dependencies). However, when bundled, such an error should never come up, since everything is in one file. Unless of course, you have separated certain parts into own bundles. I do that with vite using the rollup option manualChunks and have seen this error in certain situations.

@HaydenOrz
Copy link
Contributor Author

HaydenOrz commented Feb 20, 2024

I do that with vite using the rollup option manualChunks and have seen this error in certain situations.

I made a similar attempt, but I didn't find this issue in the vite project. This issue doesn't seem to be related to chunk distribution. Currently I'm only reproducing this issue in production mode for webpack. And when I turned off webpack's optimization.minimize option, this issue no longer appears. In addition, when Webpack builds an application, code compression occurs after chunk distribution.

So for now, I'm simply thinking that the issue is only due to webpack's built-in code compression plugin. With this as a premise, I made some attempts. I noticed that antlr4ng uses esbuild to build the release package, and the minify option is turned on at build time. And by default, webpack uses the TerserWebpackPlugin to compress the code. And it supports the use of esbuild for compression.

Then, I did the following in the demo mentioned above:

Installed dependencies

npm install esbuild terser-webpack-plugin -D

Modified webpack.prod.config.js

const devConfig = require('./webpack.config');
const TerserPlugin = require('terser-webpack-plugin');

delete devConfig.devServer;
delete devConfig.devtool;

// devConfig.devtool = 'source-map'

devConfig.mode = 'production';
devConfig.optimization = {
	minimizer: [
		new TerserPlugin({
			minify: TerserPlugin.esbuildMinify,
		}),
	],
};

module.exports = devConfig;

Then the problem no longer appears.

@HaydenOrz
Copy link
Contributor Author

I'm not familiar with code compression, but it looks like it's the result of a conflict between code compression plugins.

Also, I think antlr4ng might fix this by not turning on the minify option at build time. This requires some additional testing.

@mike-lischke
Copy link
Owner

Interesting, I didn't know that. Actually, I was thinking the other day if I should keep the minify option on, given that it doesn't make things really faster, but gets in the way when debugging test applications. So, maybe I should switch off minification...

@HaydenOrz
Copy link
Contributor Author

HaydenOrz commented Feb 20, 2024

Interesting, I didn't know that. Actually, I was thinking the other day if I should keep the minify option on, given that it doesn't make things really faster, but gets in the way when debugging test applications. So, maybe I should switch off minification...

Haha, these coincidences help you make your choice.

Also, in my opinion, a library doesn't need to be compressed, especially for the JS ecosystem, since almost all projects will have bundlers. Code compression for libraries generally only appears on CDN resources.

@HaydenOrz
Copy link
Contributor Author

I did some trying, turned off minify for antlr4ng, and tested it in the webpack project, and the results were frustrating.

The process is as follows:

Run following commands in antlr4ng:

npm run build

npm run pack

Then I got a tarball of antlr4ng, named antlr4ng-2.0.10.tgz.

Move it to the root directory of the demo project above and run the following command in the demo project:

npm install ./antlr4ng-2.0.10.tgz

Then I build the demo project and preview it, but I still get the following error:

image

@HaydenOrz
Copy link
Contributor Author

HaydenOrz commented Feb 21, 2024

Maybe it's the same issue with #34 and #31

@HaydenOrz
Copy link
Contributor Author

I made another attempt to abandon esbuild to bundle when antlr4ng build, and only compile with tsc.
Then I found that antlr4ng built like this worked well in the webpack project without any issues.

I believe the problem may be with webpack or terser, but at the moment I haven't found a good way to get antlr4ng to work with webpack and terser.

If you have any progress on this issue, please let me know, appreciated.

@HaydenOrz
Copy link
Contributor Author

HaydenOrz commented Feb 23, 2024

Hi Mike, here's good news!

I've found that turning on esbuild's --keep-names option solves this problem. This requires modifying antlr4ng's build-bundle command to

esbuild ./src/index.js --main-fields=module,main --bundle --target=esnext --keep-names

antlr4-c3 has the same problem and can be solved in the same way.

I've done quite a bit of testing in both webpack and vite projects, both in production mode and in development mode.

Enabling this option will result in a different packaging result than before it was enabled, as shown below:

  1. Two new lines of code have been added to the file header, which are used to keep the name of the class or function

    var __defProp = Object.defineProperty;
    var __name = (target, value) => __defProp(target, "name", { value, configurable: true });
  2. A static methods will be added to all classes(in the case of ParseTreeWalker):

    var ParseTreeWalker = class _ParseTreeWalker {
      static {
        __name(this, "ParseTreeWalker");
      }
      // ...
     }

If you accept the above changes, I'm willing to create a PR. In addition, I can provide a demo repository with all my test cases if you need it.

@mike-lischke
Copy link
Owner

Hey @HaydenOrz, thanks for the PR. Hopefully this has no impact on the execution speed.

@HaydenOrz
Copy link
Contributor Author

Hey @HaydenOrz, thanks for the PR. Hopefully this has no impact on the execution speed.

These new static blocks of code will only be executed when the class is initialized, i.e., only once. So I don't think this will have any impact on the execution speed of antlr4ng.

Also, I'd like to ask, do you have any plans to release a new version in the near future, I need this change to make anltr4ng integrate smoothly in my library. The same goes for antlr4-c3.

@mike-lischke
Copy link
Owner

If it's urgent I can try to set out new releases today.

@HaydenOrz
Copy link
Contributor Author

If it's urgent I can try to set out new releases today.

That couldn't be better

I need to finish switching at runtime within this week.

Also, I created the same PR in antlr4-c3 and solved the same issue. I hope that antlr4-c3 can be upgraded to a new version of antlr4ng and that antlr4-c3 will also be released.

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

2 participants