Feature/monorepo support

[thefill]

Improvement of the detection of node_modules dir - adds bottom-up traversing to find it in parent dirs.

This should resolve some issues experienced with monorepos (e.g. with nx).

Example of an issue this can fix: #820 (comment)

To resolve most of the monorepo-related issues, we would need to refactor src/wranglerjs/mod.rs a bit more to establish the common root of node project (package.json and node_module location) and share it across a few other functions - still, that's a bit more work. I can do this some other time if you are keen on that.

[thefill]

Another issue that can be resolved by this PR: #482

Limitation: As long as there is an existing "webpack.config.json" in the worker dir, users can successfully execute any cli subcommands (dev, build etc.) without triggering local "npm install". If there is none cli will be seeking package.json.

[ags799]

Thanks!

[thefill]

@ags799 np, glad I could be of help!

[thefill]

@ags799 any timeline for this change to be released? Not rushing things - just want to use this in a monorepo!

[ags799]

Within a week!

…

On Thu, Feb 11, 2021 at 11:34 AM Filip Dabrowski ***@***.***> wrote: @ags799 <https://github.com/ags799> any timeline for this change to be released? Not rushing things - just want to use this in a monorepo! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1748 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAWSCDPSPHXSTW277FNRT6TS6QIMBANCNFSM4W4M2QQA> .

[thefill]

@ags799 amazing, thanks for an update!

[ags799]

@thefill FYI the power outages in Texas have affected several of our engineers. The release could be delayed for another week.

[thefill]

@thefill FYI the power outages in Texas have affected several of our engineers. The release could be delayed for another week.

oh my, I have heard it's harsh there -_- no worries & thanks for letting me know.

[xortive]

I think we need to revert this PR, unfortunately. It's causing issues for the cloudflare-docs repo (and I think all workers-sites projects), I believe because it's detecting the node_modules in the root when attempting to run npm install in the workers-site directory.

@thefill If you could work on a fix, I can take a look at it.

[thefill]

oh snap, sure will have a look ;-)

[thefill]

I think we need to revert this PR, unfortunately. It's causing issues for the cloudflare-docs repo (and I think all workers-sites projects), I believe because it's detecting the node_modules in the root when attempting to run npm install in the workers-site directory.

@thefill If you could work on a fix, I can take a look at it.

Had a look at the cloudflare-docs@master and on the surface looks like a very simple setup - with nested package.json files, which should not cause problems. But obv the actions themselves have few more steps so I sure I'm missing things.

Any chance you can point me to the build that is failing (you have quite a few failures in actions across the last few days - not sure all relate to the issue).

[thefill]

I think we need to revert this PR, unfortunately. It's causing issues for the cloudflare-docs repo (and I think all workers-sites projects), I believe because it's detecting the node_modules in the root when attempting to run npm install in the workers-site directory.
@thefill If you could work on a fix, I can take a look at it.

Had a look at the cloudflare-docs@master and on the surface looks like a very simple setup - with nested package.json files, which should not cause problems. But obv the actions themselves have few more steps so I sure I'm missing things.

Any chance you can point me to the build that is failing (you have quite a few failures in actions across the last few days - not sure all relate to the issue).

ok, I see where the problem is - all clear. We need to make it a bit more explicit in detecting its node_modules by correlating it with the location of the closest package.json.

This should follow the principle of most node projects as by design node_module should be located on the same level as package.json

[thefill]

So the logic will go as follows:

• Search for the closest package.json
• if found check if there is node_module in the same dir - if yes all is honky dory
• else follow the current fallback (npm i etc)

That's just a quick draft - I'm sure this will change a bit when having code in front of my face.

[LeoColomb]

• else follow the current fallback (npm i etc)

Then we lose the "monorepo" support, right? It's common to have a package.json alone, and another one above in the tree with the node_modules directory.

[thefill]

• else follow the current fallback (npm i etc)

Then we lose the "monorepo" support, right? It's common to have a package.json alone, and another one above in the tree with the node_modules directory.

Ah, you mean orphaned package.json for e.g. release purposes? Uhh yeah, that's a good point.

It all comes back to the question - should wrangler CLI be capable of handling all possible monorepo configs? It's an important one as I have seen in the past mono repo where you have node_modules on different levels - e.g. root provides tools, below one provides specific dependencies. Obv node resolves this but this CLI also have the job of installing wranglerjs module so where in that case it should do this?

This begs for the installation of wranglerjs not during the execution of the "wrangler build" but rather during CLI installation e.g. during "npm i @cloudflare/wrangler -g"

The flow could be as follows:

• user installs wrangler globally
• wrangler installs as a sidecar wranglerjs in its dir
• when user executes "wrangler build" / "wrangler dev"
• wrangler uses its own wranglerjs version

This would resolve many potential monorepo problems like dir structure or tooling type (yarn / npm / pnpm).

[xortive]

I think #1677/custom builds will support the monorepo use case, since it doesn't require wranglerjs, and therefore doesn't automatically run npm install.

If you agree, I think we should revert this and release 1.14.2. Once #1677 makes it out into a release candidate which should be soon, folks using monorepos can convert to using custom builds.

[thefill]

I think #1677/custom builds will support the monorepo use case, since it doesn't require wranglerjs, and therefore doesn't automatically run npm install.

If you agree, I think we should revert this and release 1.14.2. Once #1677 makes it out into a release candidate which should be soon, folks using monorepos can convert to using custom builds.

yep, I think that's the safest way going forward - trying to guess what different projects needs is pointless - we will always find an edge case.