web-infra-dev / rspack

The fast Rust-based web bundler with webpack-compatible API 🦀️
https://rspack.dev
MIT License
9.72k stars 559 forks source link

[Feature]: provide better comment annotation #7591

Open hardfist opened 2 months ago

hardfist commented 2 months ago

What problem does this feature solve?

currently rspack | webpack will generate the following result for modern-module, the comment is not nicely(Concate module seems hard to understand for normal users) to read, even minifier can remove these comments, but normally library code shouldn't be minified for better debug experience

;// CONCATENATED MODULE: ./src/answer.js
const answer = 42;

;// CONCATENATED MODULE: ./src/secret.js
const secret = 'rspack';

;// CONCATENATED MODULE: ./src/index.js

export { answer, secret };

//# sourceMappingURL=index.mjs.map

What does the proposed API of configuration look like?

there're some solutions, i'm not sure which is better

hardfist commented 2 months ago

@alexander-akait is this option acceptable for webpack or any better suggestions for this problem? also cc @fi3ework

chenjiahan commented 2 months ago

The original module path (./src/answer.js) is useful for debugging, maybe we can replace CONCATENATED MODULE with another words, or only remove CONCATENATED MODULE.

chenjiahan commented 2 months ago

We can consider adding an rspackFuture.friendlyComments option to generate more user friendly comments, and it can be enabled in Rspack v2 by default.

hardfist commented 2 months ago

We can consider adding an rspackFuture.friendlyComments option to generate more user friendly comments, and it can be enabled in Rspack v2 by default.

comments content shouldn't be public api, so change comment content seems not a breaking change

chenjiahan commented 2 months ago

Yes, if Rspack doesn't strive for consistency with webpack's outputs and comments, then changing the comments content directly is a better idea.

alexander-akait commented 2 months ago

I am fine to improve comment, maybe we can do it without any new options for output, it is just a debug infromation, so we can just impove it

alexander-akait commented 2 months ago

How do you want to see them?

hardfist commented 2 months ago

How do you want to see them?

to me, the esbuild's comment is the simplest and clean

➜  webpack-demo esbuild src/index.mjs --bundle      
"use strict";
(() => {
  // src/answer.mjs
  var answer = 42;

  // src/secret.mjs
  var secret = "rspack";

  // src/index.mjs
  console.log({
    answer,
    secret
  });
})();

the module path is already contains enough info for debugging and seems no need to keep the CONCATENATED MODULE:(which is hard for users who don't know webpack's internal)

alexander-akait commented 2 months ago

So you just want to remove CONCATENATED MODULE words?

fi3ework commented 2 months ago

So you just want to remove CONCATENATED MODULE words?

Yes, that will sufficient.

And it will be nicer to adding the leading ; introduced in https://github.com/webpack/webpack/pull/11920 only when necessary, if possible.

alexander-akait commented 2 months ago

@fi3ework @hardfist Feel free to send a Pull requests

And it will be nicer to adding the leading ; introduced in https://github.com/webpack/webpack/pull/11920 only when necessary, if possible.

I think we already insert it only when needed, if not, consider it as a bug fix

stale[bot] commented 2 weeks ago

This issue has been automatically marked as stale because it has not had recent activity. If this issue is still affecting you, please leave any comment (for example, "bump"). We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

fi3ework commented 2 weeks ago

bump, already landed in webpack.