Skip to content

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Oct 3, 2025

Pure refactor. Makes only minor cosmetic changes to the generated raw transfer deserializer code, mainly just alters how that code is generated.

Previously we generated the source for the 2 different deserializers (JS and TS) separately, with the JS deserializer leaving out some fields e.g. typeAnnotation.

Instead, generate only 1 base implementation, with TS-only fields gated by IS_TS. IS_TS is defined as a const at top of the file.

const IS_TS = true;

function deserializeFunction(pos) {
  const params = deserializeBoxFormalParameters(pos + 56);
  if (IS_TS) {
    const thisParam = deserializeOptionBoxTSThisParameter(pos + 48);
    if (thisParam !== null) params.unshift(thisParam);
  }

  return {
    type: deserializeFunctionType(pos + 84),
    params,
    ...(IS_TS && {
      declare: deserializeBool(pos + 87),
      typeParameters: deserializeOptionBoxTSTypeParameterDeclaration(pos + 40),
    }),
    // ... etc ...
  };
}

Then we generate both the JS and TS deserializers from this same file, by setting the IS_TS const to true or false, and using minifier to shake out the dead code.

This:

  1. Simplifies custom deserializers.
  2. Opens the door to add more const flags, e.g. RANGES to switch on/off including range field in AST nodes.

@github-actions github-actions bot added A-parser Area - Parser A-ast Area - AST A-ast-tools Area - AST tools C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Oct 3, 2025
Copy link
Member Author

overlookmotel commented Oct 3, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 3, 2025

CodSpeed Instrumentation Performance Report

Merging #14312 will not alter performance

Comparing 10-03-refactor_napi_parser_use_minifier_to_generate_js_ts_raw_transfer_deserializers_from_single_source (34e1c0b) with main (f37b211)1

Summary

✅ 37 untouched

Footnotes

  1. No successful run was found on main (34e1c0b) during the generation of this report, so f37b211 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@overlookmotel overlookmotel marked this pull request as ready for review October 3, 2025 12:53
@overlookmotel overlookmotel self-assigned this Oct 3, 2025
@overlookmotel overlookmotel force-pushed the 10-03-refactor_napi_parser_use_minifier_to_generate_js_ts_raw_transfer_deserializers_from_single_source branch from 0b616e8 to 1fbda58 Compare October 3, 2025 13:31
@overlookmotel overlookmotel force-pushed the 10-02-refactor_napi_parser_minify_syntax_in_raw_transfer_deserializers branch from 8dea470 to 066f4a5 Compare October 4, 2025 12:36
@overlookmotel overlookmotel force-pushed the 10-03-refactor_napi_parser_use_minifier_to_generate_js_ts_raw_transfer_deserializers_from_single_source branch from 1fbda58 to 77bc09c Compare October 4, 2025 12:36
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Oct 4, 2025
Copy link
Member Author

overlookmotel commented Oct 4, 2025

Merge activity

…serializers from single source (#14312)

Pure refactor. Makes only minor cosmetic changes to the generated raw transfer deserializer code, mainly just alters *how* that code is generated.

Previously we generated the source for the 2 different deserializers (JS and TS) separately, with the JS deserializer leaving out some fields e.g. `typeAnnotation`.

Instead, generate only 1 base implementation, with TS-only fields gated by `IS_TS`. `IS_TS` is defined as a `const` at top of the file.

```js
const IS_TS = true;

function deserializeFunction(pos) {
  const params = deserializeBoxFormalParameters(pos + 56);
  if (IS_TS) {
    const thisParam = deserializeOptionBoxTSThisParameter(pos + 48);
    if (thisParam !== null) params.unshift(thisParam);
  }

  return {
    type: deserializeFunctionType(pos + 84),
    params,
    ...(IS_TS && {
      declare: deserializeBool(pos + 87),
      typeParameters: deserializeOptionBoxTSTypeParameterDeclaration(pos + 40),
    }),
    // ... etc ...
  };
}
```

Then we generate both the JS and TS deserializers from this same file, by setting the `IS_TS` const to `true` or `false`, and using minifier to shake out the dead code.

This:

1. Simplifies custom deserializers.
2. Opens the door to add more `const` flags, e.g. `RANGES` to switch on/off including `range` field in AST nodes.
@graphite-app graphite-app bot force-pushed the 10-02-refactor_napi_parser_minify_syntax_in_raw_transfer_deserializers branch from 066f4a5 to a98757a Compare October 4, 2025 14:11
@graphite-app graphite-app bot force-pushed the 10-03-refactor_napi_parser_use_minifier_to_generate_js_ts_raw_transfer_deserializers_from_single_source branch from 77bc09c to 34e1c0b Compare October 4, 2025 14:12
Base automatically changed from 10-02-refactor_napi_parser_minify_syntax_in_raw_transfer_deserializers to main October 4, 2025 14:16
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Oct 4, 2025
@graphite-app graphite-app bot merged commit 34e1c0b into main Oct 4, 2025
27 checks passed
@graphite-app graphite-app bot deleted the 10-03-refactor_napi_parser_use_minifier_to_generate_js_ts_raw_transfer_deserializers_from_single_source branch October 4, 2025 14:17
graphite-app bot pushed a commit that referenced this pull request Oct 4, 2025
…4313)

Pure refactor. Follow-on after #14312.

Simplify the raw transfer codegen. This change does not alter content of the generated functions, only the order in which they're output in generated code.
graphite-app bot pushed a commit that referenced this pull request Oct 6, 2025
…4372)

Pure refactor.

Codegen for raw transfer contains logic for generating multiple deserializer variants from a single base implementation, using feature flags to gate certain code, and minifier to shake out dead code in each (#14312).

Abstract this functionality into a `VariantGenerator` trait, and move it into the `output` module. This allows reusing the same functionality for other generators.
@Boshen Boshen mentioned this pull request Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-ast-tools Area - AST tools A-parser Area - Parser C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants