-
-
Notifications
You must be signed in to change notification settings - Fork 357
fix(metro): Fix sourceMapString is not a function, support metro@0.80.10
#4004
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
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86d6d2c+dirty | 1291.62 ms | 1296.80 ms | 5.18 ms |
| e2b64fe+dirty | 1285.78 ms | 1297.56 ms | 11.78 ms |
| 9433f35+dirty | 1232.24 ms | 1232.74 ms | 0.50 ms |
| 4a6664f+dirty | 1218.77 ms | 1221.07 ms | 2.30 ms |
| 80b2ce3+dirty | 1245.12 ms | 1262.04 ms | 16.92 ms |
| dadc233+dirty | 1266.52 ms | 1282.55 ms | 16.03 ms |
| f06c879+dirty | 1285.14 ms | 1285.86 ms | 0.72 ms |
| 5a22220+dirty | 1246.18 ms | 1249.61 ms | 3.43 ms |
| 3853f43+dirty | 1271.74 ms | 1278.04 ms | 6.30 ms |
| 8900e1a+dirty | 1268.36 ms | 1273.04 ms | 4.68 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86d6d2c+dirty | 2.92 MiB | 3.37 MiB | 464.31 KiB |
| e2b64fe+dirty | 2.92 MiB | 3.41 MiB | 499.97 KiB |
| 9433f35+dirty | 2.92 MiB | 3.41 MiB | 503.55 KiB |
| 4a6664f+dirty | 2.92 MiB | 3.60 MiB | 702.09 KiB |
| 80b2ce3+dirty | 2.92 MiB | 3.40 MiB | 492.75 KiB |
| dadc233+dirty | 2.92 MiB | 3.40 MiB | 492.53 KiB |
| f06c879+dirty | 2.92 MiB | 3.44 MiB | 533.24 KiB |
| 5a22220+dirty | 2.92 MiB | 3.48 MiB | 575.81 KiB |
| 3853f43+dirty | 2.92 MiB | 3.41 MiB | 503.54 KiB |
| 8900e1a+dirty | 2.92 MiB | 3.39 MiB | 485.96 KiB |
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86d6d2c+dirty | 1267.55 ms | 1286.21 ms | 18.66 ms |
| e2b64fe+dirty | 1232.22 ms | 1255.20 ms | 22.98 ms |
| 9433f35+dirty | 1246.94 ms | 1271.45 ms | 24.52 ms |
| 4a6664f+dirty | 1209.49 ms | 1208.63 ms | -0.86 ms |
| 80b2ce3+dirty | 1265.92 ms | 1268.60 ms | 2.69 ms |
| dadc233+dirty | 1223.20 ms | 1236.88 ms | 13.68 ms |
| f06c879+dirty | 1252.64 ms | 1259.66 ms | 7.02 ms |
| 5a22220+dirty | 1209.49 ms | 1220.94 ms | 11.45 ms |
| 3853f43+dirty | 1221.82 ms | 1242.64 ms | 20.82 ms |
| 8900e1a+dirty | 1210.27 ms | 1218.66 ms | 8.39 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86d6d2c+dirty | 2.36 MiB | 2.82 MiB | 462.82 KiB |
| e2b64fe+dirty | 2.36 MiB | 2.85 MiB | 495.80 KiB |
| 9433f35+dirty | 2.36 MiB | 2.85 MiB | 499.80 KiB |
| 4a6664f+dirty | 2.36 MiB | 3.04 MiB | 696.39 KiB |
| 80b2ce3+dirty | 2.36 MiB | 2.84 MiB | 486.98 KiB |
| dadc233+dirty | 2.36 MiB | 2.84 MiB | 486.85 KiB |
| f06c879+dirty | 2.36 MiB | 2.88 MiB | 530.42 KiB |
| 5a22220+dirty | 2.36 MiB | 2.92 MiB | 570.21 KiB |
| 3853f43+dirty | 2.36 MiB | 2.85 MiB | 499.81 KiB |
| 8900e1a+dirty | 2.36 MiB | 2.83 MiB | 479.25 KiB |
src/js/tools/vendor/metro/utils.ts
Outdated
| let sourceMapStringFunction; | ||
| if (typeof sourceMapString === 'function') { | ||
| sourceMapStringFunction = sourceMapString; | ||
| } else if (typeof (sourceMapString as NewSourceMapStringExport).sourceMapString === 'function') { |
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.
can (sourceMapString as NewSourceMapStringExport) be undefined here in a future version of metro/RN?
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.
Yes, that's a good one. Ill add a check for it.
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 27ef4ee | 317.40 ms | 321.70 ms | 4.30 ms |
| 6e8584e | 447.10 ms | 474.71 ms | 27.61 ms |
| 148f924 | 492.65 ms | 500.28 ms | 7.63 ms |
| 34aba08 | 328.10 ms | 342.84 ms | 14.74 ms |
| 70e6261 | 482.65 ms | 495.70 ms | 13.05 ms |
| 575f9da | 415.26 ms | 422.98 ms | 7.72 ms |
| 1c65324 | 426.37 ms | 460.36 ms | 33.99 ms |
| 15c80ab+dirty | 336.27 ms | 350.58 ms | 14.31 ms |
| 1d86dd6 | 405.14 ms | 411.06 ms | 5.92 ms |
| 31fcca2 | 391.22 ms | 414.78 ms | 23.56 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 27ef4ee | 17.73 MiB | 19.82 MiB | 2.08 MiB |
| 6e8584e | 17.73 MiB | 19.86 MiB | 2.12 MiB |
| 148f924 | 17.73 MiB | 19.94 MiB | 2.21 MiB |
| 34aba08 | 17.73 MiB | 19.80 MiB | 2.07 MiB |
| 70e6261 | 17.73 MiB | 19.94 MiB | 2.21 MiB |
| 575f9da | 17.73 MiB | 19.83 MiB | 2.10 MiB |
| 1c65324 | 17.73 MiB | 19.95 MiB | 2.21 MiB |
| 15c80ab+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
| 1d86dd6 | 17.73 MiB | 19.86 MiB | 2.12 MiB |
| 31fcca2 | 17.73 MiB | 19.90 MiB | 2.17 MiB |
lucas-zimerman
left a comment
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.
LGTM!
* Bumps cocoa from 8.29.1 to 8.36.0 * removed duplicate SentryTraceHeader definition * Update CHANGELOG.md * fix(metro): Fix sourceMapString is not a function, support metro@0.80.10 (see #4004) * misc: Update to RN 0.73.9, fix CI flipper error (see #3981) * use fixed version of react-native-gesture-handler --------- Co-authored-by: lucas <lucas-zimerman1@hotmail.com>
📢 Type of change
📜 Description
This PR fixes
sourceMapStringis not a function error which happens due to changes inmetro@0.80.10.The changes facebook/metro@v0.80.9...v0.80.10#diff-1b836d1729e527a725305eef0cec22e44605af2700fa413f4c2489ea1a03aebcL28
At
metro@0.80.9At
metro@0.80.10This PR also adds a new error message in case the expor would change in the future.
💚 How did you test it?
sample app, ci
📝 Checklist
sendDefaultPIIis enabled