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

feat(metro-config): add debug id support #25935

Merged
merged 12 commits into from
Dec 15, 2023

Conversation

EvanBacon
Copy link
Contributor

Why

Injecting the debugId is a complex operation that doesn't fit nicely to any established plugin systems due to the way it requires a content hash, followed by a conversion to hermes bytecode. Other modern bundlers, specifically bun, also provide the debugId annotation, even though it seems to be primarily leveraged by the creators of the semantic––Sentry.

By adding this upstream, we can expose simple hooks for sentry to inject virtual runtime modules that access the debugId while leaving the more complex operation to expo/metro-config (which has robust testing).

How

Add two implementations of the debugId feature:

  1. multi-asset serializer: this is used in npx expo export
  2. legacy serializer: used in npx expo export:embed

Both follow a similar semantic of creating a debugId before the final bundle is processed. Neither serializer injects any debug Id info when inline source maps are enabled or when source maps are disabled.

In the multi-asset serializer, we inject the debug Id in the hermes source maps too. This is currently unused but will be leveraged for React Server Components in the future.

We may need a follow up PR like #25859 to expose hooks for sentry to inject runtime modules.

Test Plan

Only tested against the internal suite, this may have inconsistencies against various production native builds but that seems unlikely given the heavy restrictions we've placed on modifying the metro serializer.

@EvanBacon EvanBacon requested review from quinlanj and removed request for Simek and marklawlor December 14, 2023 05:59
@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Dec 14, 2023
EvanBacon and others added 2 commits December 14, 2023 00:01
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Dec 14, 2023

The `debugId` is a deterministic hash of the bundle's contents without the external bundle splitting references. This is effectively the same value used to create a chunks filename, but formatted as a UUID, e.g. `431b98e2-c997-4975-a3d9-2987710abd44`.

`@expo/metro-config` injects `debugId` during `npx expo export` and `npx expo export:embed`. Any additional optimization steps in `npx expo export:embed` like Hermes bytecode generation will need to have the `debugId` injected manually.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description reads well and matches the expected behavior/the current Debug IDs behavior with Sentry Serializer in bare RN apps.

Sentry currently has these guidelines https://docs.sentry.io/platforms/react-native/sourcemaps/#manual-upload-with-hermes for bare RN. We will add a similar for npx expo export:embed.

Comment on lines 528 to +535
sourceMaps: false,
},
});
expect(typeof str).toBe('string');
expect(typeof bundle).toBe('object');
// Ensure the module is run.
expect(str).toMatch(/TEST_RUN_MODULE\("\/app\/index\.js"\);/);
expect(str).toMatch(/expo-mock\/async-require/);
expect(bundle.code).toMatch(/TEST_RUN_MODULE\("\/app\/index\.js"\);/);
expect(bundle.code).toMatch(/expo-mock\/async-require/);
expect(bundle.map).toMatch(/debugId/);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unexpected to me, why is the source map with debug ID present when the sourceMaps: false option is set?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the export:embed scenario?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an issue with how export:embed works, the more recent versions don't perform additional work.

Comment on lines +377 to +378
// NOTE: Sentry does this, but bun does not.
// sourceMapObject.debug_id = debugId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirming only one attribute debugId will work.

Sentry Serializer includes both for legacy reasons, which can be omitted when implementation this feature now.

Comment on lines +97 to +110
if (config.serializer?.customSerializer) {
const bundle = await config.serializer?.customSerializer(
entryPoint,
preModules,
graph,
options
);
if (typeof bundle === 'string') {
bundleCode = bundle;
} else {
bundleCode = bundle.code;
bundleMap = bundle.map;
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default serializer is not created if there is an existing custom serializer. -> https://github.com/expo/expo/pull/25935/files#diff-a07d4017951693f251b9c918dec8b9f2a2d2896f77d212181e6d98444050eeceR172

The custom serializer might not generate a debug ID we should not try to fill in the map and generate the ID for it. As it might be loose not matching any bundle.

I'm proposing this: @evanbacon/metro-config/debug-id-support...krystofwoldrich:expo:kw-add-debug-id-to-default-serializer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine, but the sentry plugin would possibly not use this as it would only apply to one possible export path, export:embed. If we expose a more limited function like modifyDeps(preModules, { debugId }) then it will work in both code paths.

docs/pages/versions/unversioned/config/metro.mdx Outdated Show resolved Hide resolved
docs/pages/versions/unversioned/config/metro.mdx Outdated Show resolved Hide resolved
docs/pages/versions/v50.0.0/config/metro.mdx Outdated Show resolved Hide resolved
docs/pages/versions/v50.0.0/config/metro.mdx Outdated Show resolved Hide resolved
@EvanBacon EvanBacon merged commit 50a4797 into main Dec 15, 2023
20 checks passed
@EvanBacon EvanBacon deleted the @evanbacon/metro-config/debug-id-support branch December 15, 2023 03:20
@brentvatne brentvatne added the published Changes from the PR have been published to npm label Dec 15, 2023
brentvatne added a commit that referenced this pull request Dec 19, 2023
# Why

Expose experimental plugins to run before the serialization of each code
asset.

```ts
  unstable_beforeAssetSerializationPlugins?: ((serializationInput: {
    graph: ReadOnlyGraph<MixedOutput>;
    premodules: Module[];
    debugId?: string;
  }) => Module[])[];
  ```

### Implementation notes
These are different from the serialization processors which run on the entirety of the code before it is potentially bundle split. Once the code is split into their respective chunks, we run the plugins before each chunk is serialized. 


### Intended use
These plugins are intended to expose the premodules and the debugId introduced from #25935 

```js
  // metro.config.js
  const { getDefaultConfig } = require("expo/metro-config");
  
function unstable_beforeAssetSerializationPlugin({graph, premodules,
debugId}: {
    graph: ReadOnlyGraph<MixedOutput>;
    premodules: Module[];
    debugId?: string;
  }) : Module[] {
     const modifiedPremodules = modifyYourModules(premodules, debugId); 
     return modifiedPremodules;
  }

module.exports = getDefaultConfig(projectRoot,
{unstable_beforeAssetSerializationPlugins:
[unstable_beforeAssetSerializationPlugin]});
```

---------

Co-authored-by: Brent Vatne <brentvatne@gmail.com>
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
# Why

Injecting the debugId is a complex operation that doesn't fit nicely to
any established plugin systems due to the way it requires a content
hash, followed by a conversion to hermes bytecode. Other modern
bundlers, specifically bun, also provide the `debugId` annotation, even
though it seems to be primarily leveraged by the creators of the
semantic––Sentry.

By adding this upstream, we can expose simple hooks for sentry to inject
virtual runtime modules that access the debugId while leaving the more
complex operation to expo/metro-config (which has robust testing).

<!--
Please describe the motivation for this PR, and link to relevant GitHub
issues, forums posts, or feature requests.
-->

# How

Add two implementations of the debugId feature:

1. multi-asset serializer: this is used in `npx expo export`
2. legacy serializer: used in `npx expo export:embed`

Both follow a similar semantic of creating a debugId before the final
bundle is processed. Neither serializer injects any debug Id info when
inline source maps are enabled or when source maps are disabled.

In the multi-asset serializer, we inject the debug Id in the hermes
source maps too. This is currently unused but will be leveraged for
React Server Components in the future.

We may need a follow up PR like expo#25859
to expose hooks for sentry to inject runtime modules.

<!--
How did you build this feature or fix this bug and why?
-->

# Test Plan

Only tested against the internal suite, this may have inconsistencies
against various production native builds but that seems unlikely given
the heavy restrictions we've placed on modifying the metro serializer.

---------

Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
Co-authored-by: Aman Mittal <amandeepmittal@live.com>
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
# Why

Expose experimental plugins to run before the serialization of each code
asset.

```ts
  unstable_beforeAssetSerializationPlugins?: ((serializationInput: {
    graph: ReadOnlyGraph<MixedOutput>;
    premodules: Module[];
    debugId?: string;
  }) => Module[])[];
  ```

### Implementation notes
These are different from the serialization processors which run on the entirety of the code before it is potentially bundle split. Once the code is split into their respective chunks, we run the plugins before each chunk is serialized. 


### Intended use
These plugins are intended to expose the premodules and the debugId introduced from expo#25935 

```js
  // metro.config.js
  const { getDefaultConfig } = require("expo/metro-config");
  
function unstable_beforeAssetSerializationPlugin({graph, premodules,
debugId}: {
    graph: ReadOnlyGraph<MixedOutput>;
    premodules: Module[];
    debugId?: string;
  }) : Module[] {
     const modifiedPremodules = modifyYourModules(premodules, debugId); 
     return modifiedPremodules;
  }

module.exports = getDefaultConfig(projectRoot,
{unstable_beforeAssetSerializationPlugins:
[unstable_beforeAssetSerializationPlugin]});
```

---------

Co-authored-by: Brent Vatne <brentvatne@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint compatible bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants