refactor: generate currency source files

[johnhooks]

As a first step to add currencies of different TAmounts, I created scripts to generate source files from a shared json data file. This should make implementing PR #582 trivial.

Other changes

• removed all original Currency<number> source files
• added generate script to @dinero.js/currencies
• updated prepare script of @dinero.js/currencies to generate sources files before building
• added @public comment to all Currency<number> objects for api-extractor
• added @public comment to Currency<TAmount> type for api-extractor

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
dinerojs	❌ Failed (Inspect)			Feb 19, 2023 at 3:55AM (UTC)

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b9467fc:

Sandbox	Source
@dinero.js/example-cart-react	Configuration

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c144f5f:

Sandbox	Source
@dinero.js/example-cart-react	Configuration
@dinero.js/example-cart-vue	Configuration
@dinero.js/example-pricing-react	Configuration
@dinero.js/example-starter	Configuration

[johnhooks]

Since api-extractor was added before this PR, we can be petty confident that this didn't make any changes to the existing api, because it didn't modify the currencies.api.md file generated by api-extractor.

[johnhooks]

To verify the generation makes no changes to existing files, I replaced all the original source files from the main branch. After
running the generation script the replaced currency files are identical to the originals.

I modified the genCurrenciesNumber.mjs script by remove the @public comment for api-extractor. This was all that was required to make the generated code identical to the originals.

[sarahdayan]

Really cool change, that's gonna be useful.

One early thought before I review this more thoroughly—way the script is designed, if we add a genCurrenciesNumber script, we'll read and parse the JSON file twice (since we'll call genCurrencies in each script). We can avoid that by creating configuration objects instead that we pass to a generator. The generator reads and parses once, and maps over configurations to which it passes the data. That's how Rollup works, and it makes the final script easy to read and to update.

For example, we could have the entry point look like this:

const numberCurrencyConfig = createCurrencyConfig(
  ({ currency, ...description }) => ({
    ...currency,
    description,
    genericType: 'number',
  })
);

const bigintCurrencyConfig = createCurrencyConfig(
  ({ currency, ...description }) => ({
    ...currency,
    description,
    base: `${base}n`,
    exponent: `${exponent}n`,
    genericType: 'bigint',
  })
);

const configs = [numberCurrencyConfig, bigintCurrencyConfig];

export function build() {
  // load data

  configs.forEach((config) => {
    // do stuff
  });
}

[johnhooks]

Beautiful. I'll incorporate that.

[reckter]

This could easily be adopted to be even more generic (abstractions all the way!), By letting the number part (and the formatting of the numbers down below), be configured.

This is just a pop-up thought, and might just be a bit too much abstraction layering though.

[johnhooks]

I'm working through some thoughts @sarahdayan about better abstractions. I should have it up in a few days.

[johnhooks]

@reckter checkout out the abstraction now. Creating the currency generation config adds a mapNumber function which converts the JS number type of the json data file into a string that will be output into the generated source files.

Example for bigint

createCurrencyConfig({
  genericType: 'bigint',
  mapNumber: (n) => `${n}n`,
}

[johnhooks]

Eventually we would want this broken out into another layer of abstraction, so it will auto generate other amendments. But I think leaving that for another day is okay.

[johnhooks]

I'm unsure why the ci/codesandbox failed. Are all the packages built concurrently? There is a type error for all the currency files imported in the index file, stating they are not modules. I don't receive the same error locally.

[johnhooks]

I made the default to load the number currency library. Currently the API is identical to the previous, the bigint currency files aren't exported by the package.

[johnhooks]

Added this to emulate api-extractors output. It's helpful to see in the CI output. Also added cat face, if too cute I can remove it.

[johnhooks]

Merging #672 fixed the issue I was having with ci/codesandbox. I don't know exactly how. There must have been a step that was happening out of the correct sequence with the redundant build steps.

[sarahdayan]

Do you want to change the base branch to johnhooks:remove-ci-duplication to remove the unwanted diff? GitHub should automatically change it to main once #672 is merged, and this way we can review it without the noise.

[johnhooks]

Sorry @sarahdayan its only letting me choose from the difference branches of the dinerojs/dinero.js repo not my fork. A little reading on Stack Overflow and it seems like maybe I need to open another pull request. Or maybe the easiest, I can walk back the merge?

[johnhooks]

@sarahdayan are you still interested in generating currency files?

[reckter]

Is there a reason this wasn't merged in the end? Did I just not see replacement for this?

If I just completely missed it, my bad :)

But as #582 is still open, we probably need a solution for this topic

[johnhooks]

@reckter I hadn't heard from Sarah on whether or not she want to use it, so I closed it.