nestjs icon indicating copy to clipboard operation
nestjs copied to clipboard

Migrate to Nx monorepo

Open jeremylvln opened this issue 2 years ago • 12 comments

This PR converts this Lerna monorepo to a Nx one. Closes #143. Note: this PR is based on #342.

I've done no functional changes except lint fixes. I work with Nx every day, so I guess this implementation is somewhat ideal. Sorry for the large diff, a lot of files has been moved.

I've modified the CI accordingly, do not worry about the MansaGroup/nrwl-nx-action action used inside, I've created it :)

Tests are welcomed!

jeremylvln avatar Jan 21 '22 14:01 jeremylvln

@IamBlueSlime Thanks a bunch for doing this, our priority is getting v8 PR first and then this should be our next step

underfisk avatar Jan 22 '22 18:01 underfisk

@IamBlueSlime Thanks a bunch for doing this, our priority is getting v8 PR first and then this should be our next step

That's why this PR is rebased on the other one :D

jeremylvln avatar Jan 22 '22 18:01 jeremylvln

Wondering if it would be easier to just do a fresh migration over to NX now that the upgrade to Nest 8 is done. There's a huge number of duplicated commits and merge commits here that we don't really need.

WonderPanda avatar Jan 24 '22 01:01 WonderPanda

I will rebase the PR correctly asap

jeremylvln avatar Jan 24 '22 07:01 jeremylvln

Hi @WonderPanda, @underfisk. I've rebased the PR, I hope that I've not removed any commit during the process. As this PR moves a lot of file, pay attention to the modification of these file if I've missed something.

Btw, I've removed the CI job name, thus the required checks will not be filled, this PR will need to be force merged by an administrator.

jeremylvln avatar Jan 24 '22 09:01 jeremylvln

I'm not interested right now in switching away from Yarn 1 and over to NPM so we should revert the package-lock.json and changes to the CI scripts.

In the future I'd be more open to considering PNPM but it should be as a different PR and not mixed in with the NX stuff

I agree, if we have to switch our package manager I would definitely advocate for PNPM but now on this scope, we should just aim to switch to Nx with the bare minimum of what we have, and then we can start thinking about what's next

underfisk avatar Jan 28 '22 19:01 underfisk

@IamBlueSlime Can we merge the latest changes, i think we should start considering having Nx asap, there are some stuff that we have pending that can actually leverage having this ecosystem

underfisk avatar Feb 02 '22 01:02 underfisk

@IamBlueSlime Can we merge the latest changes, i think we should start considering having Nx asap, there are some stuff that we have pending that can actually leverage having this ecosystem

Hi @underfisk! There is currently an issue with Nx that breaks JS reflection on build, see https://github.com/nrwl/nx/pull/8784. I do not think that libraries are impacted, but only the apps that are build. In all cases, just to be sure, I think it would be preferable to wait until my PR on Nx is merged before merging this one.

But if you want me to rebase this PR and remove npm-related commits, I can totally do that :)

jeremylvln avatar Feb 02 '22 09:02 jeremylvln

@IamBlueSlime Can we merge the latest changes, i think we should start considering having Nx asap, there are some stuff that we have pending that can actually leverage having this ecosystem

Hi @underfisk! There is currently an issue with Nx that breaks JS reflection on build, see nrwl/nx#8784. I do not think that libraries are impacted, but only the apps that are build. In all cases, just to be sure, I think it would be preferable to wait until my PR on Nx is merged before merging this one.

But if you want me to rebase this PR and remove npm-related commits, I can totally do that :)

No rush, i've ran into a similar problem before, well at max you can override terser plugin with a custom nx webpack file but if they can include that officially it would be better

underfisk avatar Feb 02 '22 13:02 underfisk

@IamBlueSlime I've noticed that the Nx PR has been merged, any news so far if it is included in any of the latest versions?

underfisk avatar Mar 08 '22 13:03 underfisk

@IamBlueSlime Do you have any plans on having this? I believe we were close to merge this experience

underfisk avatar Apr 23 '22 16:04 underfisk

@IamBlueSlime Do you have any plans on having this? I believe we were close to merge this experience

Hey, sorry for the long time, I've been quite busy at work. I do not think I will have the time to work on this today, but maybe tomorrow or Wednesday. Is this okay for you?

jeremylvln avatar Apr 25 '22 08:04 jeremylvln