-
Notifications
You must be signed in to change notification settings - Fork 0
prepare script not working with yarn #38
Comments
Odd, what happens if you run
Apps-rendering and image-rendering consume the package from GitHub - which means they pull the source code. We could also publish an artefact if you think it would make things easier for registry consumers? |
it doesn't run the it does sound like it should though, for sure...
I guess this would have the benefit of ensuring that the build was successful before publishing too |
Based on the second bullet-point in guardian/cdk#9, perhaps @akash1810 knows something about this? |
Ah this was fun to debug! There appears to be a bug in yarn causing this and the proposed fix is scary after reading this. The changes in guardian/cdk#9 worked and I'd recommend applying the same changes here. It's worth noting that for the cdk project, we've since started publishing to npm after witnessing some strange behaviour in CI for dependant projects. That is, I'd, more strongly, recommend publishing to npm and would be interested to understand the use-case for installing from git. cc @jamie-lynch @stephengeller who travelled this journey with me and might remember more. |
Thanks Akash!
Interesting, what sort of strange behaviour? We've got a handful of projects using GH installs 🤔... it's not related to Docker containers in TC by any chance?
This repo is published to |
A general failure to install dependencies, with an unclear reasoning/stack trace, see the oldest entries here. There were also some failures on another project that's built in TeamCity which I'm unable to find the link to, sadly.
Woop! It looks like the published library only includes the original TypeScript and not the compiled JS and declaration files? That is, there's still a reliance on To test: docker run -it --rm --name test node:14.15.1 /bin/bash -c "npm install @guardian/types; ls -lah node_modules/@guardian/types" or docker run -it --rm --name test node:14.15.1 /bin/bash -c "yarn add @guardian/types; ls -lah node_modules/@guardian/types" |
Ah, yeah good point. We switched over to generating declaration files quite recently (#35), and I think the only project that uses that version is Apps-Rendering, which installs via GitHub. I believe @sndrs is planning to publish a build artifact to the registry to remedy this? So yeah, currently there is still this dependency on |
I'm really not clear what the benefit is of not publishing a build artefact? do you get something platform-specific by building each time it installs? one definite benefit of publishing the artefact (along with source code, if that's useful) is that you can fail the release process if the build fails. this is only true if building is idempotent of course... |
Oh I don't think there are any benefits to not doing it. But I don't think a build artifact is relevant for GitHub installs? Where would it be hosted? The |
Within a GH release? For example https://github.com/guardian/cdk/releases/tag/v0.3.0 ( In the guardian/cdk example, if you install from GH at |
But that's a tarball of the source isn't it, not the built artifact? How would GitHub know how to build and package your code when you create a release? |
I'm not sure if it's
yarn
-specific, but after updating to v1 in frontend, I'm not getting a dist dir:what made you go with
prepare
script over publishing an artefact?The text was updated successfully, but these errors were encountered: