-
Notifications
You must be signed in to change notification settings - Fork 79
Leverage terser to mangle/compress the generated js messages #1086
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the JS generation process to leverage terser for compressing and mangling the generated JavaScript messages instead of using prettier.
- Replaces the dependency on prettier with terser for production builds.
- Introduces a conditional minification step based on the isDebug flag.
rosidl_gen/idl_generator.js
Outdated
| code = await prettier.format(code, { parser: 'babel' }); | ||
| let result = null; | ||
| if (!isDebug && fileName.endsWith('.js')) { | ||
| result = await minify(code); |
Copilot
AI
Mar 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding error handling or logging around the terser.minify call to ensure that any minification errors are caught and handled, rather than silently falling back to the unminified code.
| result = await minify(code); | |
| try { | |
| result = await minify(code); | |
| } catch (error) { | |
| console.error(`Error minifying ${fileName}:`, error); | |
| result = null; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the code generation process to leverage terser for minifying generated JS code rather than formatting it with prettier. Key changes include:
- Removing the prettier dependency.
- Integrating terser minification with error handling for JavaScript files.
- Using a conditional debug flag (isDebug) to determine whether to minify the code.
Comments suppressed due to low confidence (2)
rosidl_gen/idl_generator.js:45
- Ensure that 'isDebug' is defined in the current scope or imported appropriately to avoid potential runtime errors during minification.
if (!isDebug && fileName.endsWith('.js')) {
rosidl_gen/idl_generator.js:55
- [nitpick] Consider assigning the minified code to the original 'code' variable before writing the file to improve clarity and maintainability.
await fse.writeFile(path.join(dir, fileName), result ? result.code : code);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR leverages terser to mangle and compress the generated JavaScript message files in place of prettier-based formatting. Key changes include:
- Replacing the prettier formatter with terser for minification.
- Removing the removeEmptyLines helper function and its invocations.
- Updating writeGeneratedCode to conditionally minify code when not in debug mode.
This patch implements to leverage [terser](https://www.npmjs.com/package/terser) to mangle/compress the generated `.js` messages when `--debug` option not added. If debugging the message files, users can run `generate-messages:dev` to check with the original files, including: 1. Removing the prettier dependency. 2. Integrating terser minification with error handling for JavaScript files. 3. Using a conditional debug flag (isDebug) to determine whether to minify the code. 4. Don't remove the blank line in the generated files because it will be handled by `terser` now. Fix: #1085
This patch implements to leverage terser to mangle/compress the generated
.jsmessages when--debugoption not added. If debugging the message files, users can rungenerate-messages:devto check with the original files, including:tersernow.Fix: #1085