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.

Sign up for GitHub

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

fix: apply custom serializer to source map endpoint in dev server #1284

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+49 −0
Open

fix: apply custom serializer to source map endpoint in dev server #1284

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions packages/metro/src/Server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1153,6 +1153,55 @@ class Server {
prepend = [];
}

// If the project defines a custom serializer, then we should run it to ensure any
// mutations to the graph are applied in the sourcemap.
Comment on lines +1156 to +1157
Copy link
Contributor

Choose a reason for hiding this comment

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

It makes sense to me to run the custom serializer consistently (and I vaguely remember seeing cases where this was broken), but what do you mean by "mutations to the graph"? When during bundling do those occur?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If someone were to preModules.splice(0,0, { ... }) to add a new module inside of a custom serializer, then it would technically mutate the arguments used to generate the bundle. I can think of two cases like this:

  • sentry (not exactly, but close)
  • One I just wrote for Expo CLI as a fallback to returning the map: here

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. The serializer API wasn't designed with side-effectful serializers in mind, and the preModules parameter specifically is typed as being read-only. Is there a way to achieve what you're trying to do without mutating read-only arguments?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah but it's a little unstable. I'm sniffing if the sourceUrl has a .map pathname to determine if the serializer should return source maps. This works for the .map endpoint, but it doesn't account for /symbolicate where the result is getExplodedSourceMap and not a standard source map string.

if (this._config.serializer.customSerializer) {
const bundle = await this._config.serializer.customSerializer(
entryFile,
prepend,
graph,
{
asyncRequireModulePath: await this._resolveRelativePath(
this._config.transformer.asyncRequireModulePath,
{
relativeTo: 'project',
resolverOptions,
transformOptions,
},
),
processModuleFilter: this._config.serializer.processModuleFilter,
createModuleId: this._createModuleId,
getRunModuleStatement:
this._config.serializer.getRunModuleStatement,
includeAsyncPaths: graphOptions.lazy,
dev: transformOptions.dev,
projectRoot: this._config.projectRoot,
modulesOnly: serializerOptions.modulesOnly,
runBeforeMainModule:
this._config.serializer.getModulesRunBeforeMainModule(
path.relative(this._config.projectRoot, entryFile),
),
runModule: serializerOptions.runModule,
sourceMapUrl: serializerOptions.sourceMapUrl,
sourceUrl: serializerOptions.sourceUrl,
inlineSourceMap: serializerOptions.inlineSourceMap,
serverRoot:
this._config.server.unstable_serverRoot ??
this._config.projectRoot,
shouldAddToIgnoreList: (module: Module<>) =>
this._shouldAddModuleToIgnoreList(module),
getSourceUrl: (module: Module<>) =>
this._getModuleSourceUrl(module, serializerOptions.sourcePaths),
},
);

// If the serializer returned the source map then we can use it here, otherwise
// fall back to the default source map generation with the possibly mutated graph.
if (typeof bundle !== 'string' && bundle.map) {
return bundle.map;
}
}

return await sourceMapStringNonBlocking(
[...prepend, ...this._getSortedModules(graph)],
{
Expand Down