Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Cleanup of generator.js and moves out Babel serialization logic #2306

Closed

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Jul 23, 2018

Release notes: none

This cleans up generator.js in the following ways:

  • pulls our NameGenerator and PreludeGenerator into it's own module
  • takes out the Babel coupling with Generator and puts the serialization logic into the serializer instead

This is of many PRs to help clean up the serialization/generator logic.

Copy link
Contributor

@hermanventer hermanventer left a comment

Choose a reason for hiding this comment

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

Looks like a simple refactor.

Yay, you've decreased the max cycle length for Flow from 57 to 56. Before you land this, please update scripts/detect_bad_deps.js to prevent anyone else from increasing it back to 57 again.

@@ -14,7 +14,7 @@ import type { SerializerOptions } from "../options.js";
import * as t from "@babel/types";
import generate from "@babel/generator";
import type { BabelNodeStatement, BabelNodeExpression, BabelNodeIdentifier } from "@babel/types";
import { NameGenerator } from "../utils/generator.js";
import { NameGenerator } from "../utils/PreludeGenerator";
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not put NameGenerator in its own file? Importing it from generator.js seems plausible, but importing it from PreludeGenerator.js just seems weird.

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@trueadm is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants