codedownio / aeson-typescript

Generate TypeScript definition files from your ADTs
BSD 3-Clause "New" or "Revised" License
60 stars 27 forks source link

feat(nix): allow library to be built and imported using nix #15

Closed bluekezza closed 4 years ago

bluekezza commented 4 years ago

In trying to evaluate your library the dependency on the tsc typescript compiler has been making it difficult to import it. Our build system uses nix which runs in an isolated environment where any build dependencies must be explicitly defined.

This PR provides nix files to make it easier for the package to be imported.

If someone wants to build the project they can run nix-build . which will evaluate default.nix.

If someone wants to import the package into their build pipeline they can use the package.nix and provide their own definitions for arguments required.

For example this is how your library would be incorporated into someone elses nix build:

with import ../nixpkgs {};

let aeson-typescript
  = let src = 
       fetchFromGitHub
         { owner = "codedownio";
           repo = "aeson-typescript";
           rev = "fcf4f6608b6aacb0f69b40fb881400353ea07070";
           sha256 = "0rwg3gb7dhvsnzl8wqdfwafymnbl61frl98sz55c527a12zicq51";
         };
       in 
         callPackage (import "${src}/package.nix")
            { callCabal2nix = self.callCabal2nix;
               overrideCabal = haskell.lib.overrideCabal;
               typescript = nodePackages.typescript;
             };
thomasjm commented 4 years ago

Cool! I'm a big fan of Nix in general. Just a couple thoughts:

1) tsc is only needed for tests, is there some reason you can't just disable tests for this library in your Nix build? 2) The tests currently run against a specific version of TypeScript, pinned in test/assets/package.json. Using whatever nodePackages.typescript you have in Nix might produce different results in tests. Probably not a huge deal but it would be nice to make sure the Nix-ified version doesn't have tricky differences from the normal version. What if you instead used Nix to inject npm and let the tests install the version of TypeScript they expect?

bluekezza commented 4 years ago

Having looked into this further it appears I can disable running the tests from my side with your existing master branch using:

  aeson-typescript =
    let src =
      fetchFromGitHub
        { owner = "codedownio";
          repo = "aeson-typescript";
          rev = "bcc2f856fbcecc7efa157e551a16c0cb5eabff9d";
          sha256 = "0s85gfgrgsgwkdpvndf2gqcvqgqx1v4mbg4rlf2nqyzbcfk6ywic";
         };
    in
      overrideCabal (self.callCabal2nix "aeson-typescript" src {}) (old: {
        doCheck = false;
      });

I was inclined to want to run the tests with my version of tsc so that I might 'fail fast' if my version of typescript isn't supported, but as you say this could invalidate the assertions you are making in your tests. So I agree that injecting my version of tsc is not the right way to go.

For that to be supported that would require the changes I made to test/Util.hs to check for and use any tsc already present before using yarn or npm.

So I would conclude that this PR is unnecessary.

thomasjm commented 4 years ago

Actually, I noticed that your changes to test/Util.hs aren't even needed -- there's already a mechanism to use the tsc on the PATH. You just set the environment variable CI=true. So you should actually be able to inject a tsc of your choice without any changes to this repo.