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: migrate locales to typescript #238

Merged
merged 7 commits into from
Jan 20, 2022
Merged

feat: migrate locales to typescript #238

merged 7 commits into from
Jan 20, 2022

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Jan 19, 2022

I automatically transformed the locales using the following java script.

Source (Click to expand)
  • Save this as Convert.java inside your Faker checkout.
  • Run java Convert.java using java 17
import static java.nio.charset.StandardCharsets.UTF_8;

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Files;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.TreeSet;
import java.util.stream.Collectors;

public class Convert {

    static final File root = new File("src/locales");

    public static void main(final String... args) throws IOException {
        transform(root, 0);
    }

    private static void transform(final File file, final int depth) throws IOException {
        final String name = file.getName();
        if (!file.exists() || name.equals("index.js") || name.equals("index.ts")) {
            return;
        }
        if (file.isDirectory()) {
            for (final File sub : file.listFiles()) {
                transform(sub, depth + 1);
            }
            buildIndex(file, depth);
        } else if (name.endsWith(".ts")) {
            transformTs(file);
        } else if (name.endsWith(".js")) {
            renameJs(file);
        }
    }

    private static void buildIndex(final File folder, final int depth) throws IOException {
        final TreeSet<String> files = Arrays.stream(folder.listFiles())
                .map(File::getName)
                .map(s -> s.replace(".ts", ""))
                .filter(f -> !f.startsWith("index"))
                .collect(Collectors.toCollection(TreeSet::new));

        final String name = folder.getName();


        final File index = new File(folder, "index.ts");
        Map<String, String> indexKeys = new HashMap<>();
        if (index.exists() && depth == 1) {
            indexKeys = Files.lines(index.toPath(), UTF_8)
                    .map(String::trim)
                    .map(s -> s.split(":"))
                    .filter(s -> s.length == 2)
                    // .peek(s -> System.out.println(s[0] + ": " + s[1]))
                    .collect(Collectors.toMap(s -> s[0], s -> s[1]));
        }

        try (FileWriter fw = new FileWriter(new File(folder, "index.ts"), UTF_8);
                BufferedWriter writer = new BufferedWriter(fw)) {
            if (depth == 1) {

                System.out.println("------------------------------------------------");
                System.out.println(name + "/index.ts");

                fw.write("import type { LocaleDefinition } from '../..';\n");
                for (final String file : files) {
                    fw.write("import " + file + " from './" + file + "';\n");
                }
                fw.write("\n");
                fw.write("const " + name + ": LocaleDefinition = {\n");
                fw.write("  title:" + indexKeys.get("title") + "\n");
                if (indexKeys.containsKey("separator")) {
                    fw.write("  separator:" + indexKeys.get("separator") + "\n");
                }
                for (final String file : files) {
                    fw.write("  " + file + ",\n");
                }
                fw.write("};\n");
                fw.write("\n");
                fw.write("export default " + name + ";\n");
            } else {
                final String importPath = ".." + ("/..".repeat(depth));
                fw.write("import type { LocaleDefinition } from '" + importPath + "';\n");
                for (final String file : files) {
                    if (file.equals("switch") || file.equals(name)) {
                        fw.write("import " + file + "_ from './" + file + "';\n");
                    } else {
                        fw.write("import " + file + " from './" + file + "';\n");
                    }
                }
                fw.write("\n");
                final String type = extractType(folder, depth);
                fw.write("const " + name + ": " + type + " = {\n");
                for (final String file : files) {
                    if (file.equals("switch") || file.equals(name)) {
                        fw.write("  " + file + ": " + file + "_,\n");
                    } else {
                        fw.write("  " + file + ",\n");
                    }
                }
                fw.write("};\n");
                fw.write("\n");
                fw.write("export default " + name + ";\n");
            }
        }
        final File indexJs = new File(folder, "index.js");
        if (indexJs.exists()) {
            indexJs.delete();
        }
    }

    private static String extractType(final File folder, final int depth) {
        if (depth == 0) {
            return "{ [lang: string]: LocaleDefinition }";
        }
        final String[] subPath =
                folder.getPath().substring(root.getPath().length() + 1).replace('\\', '/').split("/");
        final String type = "LocaleDefinition" + Arrays.stream(subPath)
                .skip(1).map(s -> "['" + s + "']")
                .collect(Collectors.joining());
        return type;
    }

    private static void transformTs(final File file) throws IOException {
        String content = Files.readString(file.toPath(), UTF_8);
        content = content.replace("module['exports'] =", "export default");
        content = content.replace("module.exports =", "export default");
        file.delete();
        Files.writeString(file.toPath(), content, UTF_8);
    }

    private static void renameJs(final File file) {
        file.renameTo(new File(file.getParentFile(), file.getName().replace(".js", ".ts")));
    }

}
  • Rename files JS to TS
  • Regenerate index.ts files + fix exports
  • Add type definitions to index.ts files
  • Fix type errors (Reverted as requested in feat: migrate locales to typescript #238 (comment))
  • Avoid type errors
  • Verify the data structure hasn't changed at all

I recommend reviewing the PR commit by commit instead of all at once.

Tests that nothing has been changed

  1. Checkout the main branch and run:
import * as index from "@faker-js/faker/src/locales";
console.log(JSON.stringify(index));

via pnpm esno index.ts > output_old.json

  1. Then checkout this PR and it again

via pnpm esno index.ts > output_new.json

  1. Sort both jsons by keys.
  2. Compare output_old.json with output_new.json
  3. Verify that output_new does not remove or rename any values and only adds new ones

Accidental fixes

  • cell_phone: require('./phone_number'),
  • nb_NO: I renamed the femine and masculine first name files to match the other languages, but I also had to change the name template because it uses these files.

@netlify
Copy link

netlify bot commented Jan 19, 2022

✔️ Deploy Preview for vigilant-wescoff-04e480 ready!

🔨 Explore the source changes: 58b1cd1

🔍 Inspect the deploy log: https://app.netlify.com/sites/vigilant-wescoff-04e480/deploys/61e940d03e58cc0007a644d3

😎 Browse the preview: https://deploy-preview-238--vigilant-wescoff-04e480.netlify.app

@Shinigami92
Copy link
Member

As I yesterday evening said in Discord, we need to fix the types and maybe some keys later in another PR
Theoretically someone from outside could call faker.fake with anything like streets and we would break their code.
So for the sake of simplicity please revert your third commit and use just : any = instead

@ST-DDT
Copy link
Member Author

ST-DDT commented Jan 20, 2022

@Shinigami92: Reverted

@Shinigami92
Copy link
Member

Seems there is an issue with locales/cz/name/first_name

@ST-DDT ST-DDT marked this pull request as ready for review January 20, 2022 09:44
@ST-DDT ST-DDT requested a review from a team as a code owner January 20, 2022 09:44
@Shinigami92 Shinigami92 marked this pull request as draft January 20, 2022 10:03
@Shinigami92 Shinigami92 marked this pull request as ready for review January 20, 2022 10:47
Shinigami92
Shinigami92 previously approved these changes Jan 20, 2022
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

I worked together with @ST-DDT via screenshare on this PR

@Shinigami92 Shinigami92 requested a review from a team January 20, 2022 10:52
@ST-DDT
Copy link
Member Author

ST-DDT commented Jan 20, 2022

I briefly scanned all commits again and compared the data tree again and found a typo in one of my manual changes.
I have fixed that and now everything should be "the same as before".

@ST-DDT
Copy link
Member Author

ST-DDT commented Jan 20, 2022

It's past 12 o'clock, so the one fixed random test fails on windows.
See also: #224

Copy link
Member

@prisis prisis left a comment

Choose a reason for hiding this comment

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

diff and test looks good

Ps.: did you write the script in Java? :D

@Shinigami92 Shinigami92 merged commit d4cfa3c into faker-js:main Jan 20, 2022
@ST-DDT ST-DDT deleted the feature/ts/locales branch January 20, 2022 11:56
pkuczynski pushed a commit to pkuczynski/faker that referenced this pull request Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants