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

metro-serializer-esbuild: /*#__PURE__*/ markers are not respected #2365

Closed
1 of 5 tasks
tido64 opened this issue Apr 14, 2023 · 2 comments
Closed
1 of 5 tasks

metro-serializer-esbuild: /*#__PURE__*/ markers are not respected #2365

tido64 opened this issue Apr 14, 2023 · 2 comments
Labels
bug Something isn't working feature: metro This is related to Metro

Comments

@tido64
Copy link
Member

tido64 commented Apr 14, 2023

What happened?

/*#__PURE__*/ markers are stripped out by Metro (in metro-transform-worker) and never reaches esbuild.

Metro hard-codes the option to strip out comments so there is currently no clean way to change this. Verified that replacing the three instances of comments: false with comments: true fixes the issue.

Docs: https://babeljs.io/blog/2018/08/27/7.0.0#pure-annotation-support

Affected Package

@rnx-kit/metro-serializer-esbuild

Version

*

Which platforms are you seeing this issue on?

  • Android
  • iOS
  • macOS
  • Windows

System Information

n/a

Steps to Reproduce

Add react-native-icons and bundle:

diff --git a/packages/test-app/package.json b/packages/test-app/package.json
index eb0bdcbb..4f134247 100644
--- a/packages/test-app/package.json
+++ b/packages/test-app/package.json
@@ -25,6 +25,7 @@
     "start": "react-native rnx-start"
   },
   "dependencies": {
+    "@warren-ms/react-native-icons": "2.0.199",
     "react": "18.2.0",
     "react-native": "^0.71.0",
     "react-native-macos": "^0.71.0",
diff --git a/packages/test-app/src/App.native.tsx b/packages/test-app/src/App.native.tsx
index 1345c498..94911607 100644
--- a/packages/test-app/src/App.native.tsx
+++ b/packages/test-app/src/App.native.tsx
@@ -1,4 +1,5 @@
 import { acquireTokenWithScopes } from "@rnx-kit/react-native-auth";
+import { Add16Regular } from "@warren-ms/react-native-icons/lib/sizedIcons/chunk-0";
 import React, { useCallback, useMemo, useState } from "react";
 import {
   NativeModules,
@@ -106,7 +107,7 @@ function useStyles() {
         marginStart: margin,
       },
     });
-  }, [colorScheme]);
+  }, [colorScheme, Add16Regular]);
 }

 function Button({ children, onPress }: ButtonProps) {

Code of Conduct

  • I agree to follow this project's Code of Conduct
@tido64
Copy link
Member Author

tido64 commented Apr 17, 2023

Upstream fix: facebook/metro#967

facebook-github-bot pushed a commit to facebook/metro that referenced this issue Apr 24, 2023
Summary:
Changelog:
* **[Feature]**: Preserve comments in unminified builds, while continuing to strip all comments from minified builds.

Add support for preserving comments in code. Useful for preserving `/*#__PURE__*/` annotations that aid tree shaking.

NOTE: In a future version of Metro, the default minifier options will change to align with the underlying minifier's default, e.g. `comments: 'some'` in Terser.

Pull Request resolved: #967

Test Plan:
Test case here: microsoft/rnx-kit#2365

|        |    Size |
| :----- | ------: |
| Before | 1567764 |
| After  | 1258413 |
| Diff   | -309351 |

Reviewed By: jacdebug

Differential Revision: D45223689

Pulled By: motiz88

fbshipit-source-id: 89c1e4050e67c3c34ec74015986827425b5fc76c
@tido64
Copy link
Member Author

tido64 commented Apr 24, 2023

Merged in 8d8f317.

@tido64 tido64 closed this as completed Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature: metro This is related to Metro
Projects
None yet
Development

No branches or pull requests

1 participant