fix: scan entries when the root is in node_modules

[Jinjiang]

Description

Fixed optimizer entries in the case the root is in node_modules

Additional context

#15613 (reply in thread)

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines, especially the Pull Request Guidelines.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Update the corresponding documentation if needed.
• Ideally, include relevant tests that fail without this PR but pass with it.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[patak-dev]

I think we could merge this PR. We should do the same for **/${config.build.outDir}/**, in that case, so we also avoid potential problems if outDir is part of the parent too. And the same for **/__tests__/** and **/__coverage__/** just to be consistent.

I'd like to see what other maintainers think about this too. Because even if your reproduction is working after this change, it may later fail unexpectedly because as I commented here, our current isInNodeModules util does a simple .includes('node_modules'). So it may not make sense to merge this PR until we have proper tests for an app inside node_modules and this util has been reworked.

There is also a higher level discussion about the extra complexity we are introducing to support apps inside node_modules. I understand that configuring this in Bit isn't possible though so if the code changes aren't hard to maintain I'm leaning towards supporting this use case in core. I think you'll have a lot of issues in the ecosystem though, because there are a lot of plugins out there that have the same kind of checks to decide if something is a dependency file or not. It could be an uphill battle. So I would still recommend to check if there is any chance to configure Bit in a way were the Apps parent folder isn't node_modules.

[Jinjiang]

@patak-dev thanks for the information. Ya, I agree in a larger picture there might be somewhere else I'm not familiar with having the similar potential ones.

On Bit's side, in short term, I'm afraid it's hard to change since the concept of Bit is that everything runs as a package, so inevitably the runtime happens in a node_modules directory. For other libs/plugins in the ecosystem, we also did some related work. By far, this issue is the last one. 😅

In the long-term, I'd like to bring this topic to the team. /cc @GiladShoham

Thanks.

[Jinjiang]

@patak-dev I've updated the code to cover other ignored patterns in globEntries().

However, I met a CI error on Windows said ResourceUnavailable: D:\a\_temp\9a25bc22-181a-46f2-b736-ec7ee0a6aa41.ps1:3. I don't quite understand this error and it seems not related to what I've changed. Do you have any ideas on this? Neither not sure a re-run would help for which I don't have that permission.

🙏

[patak-dev]

Don't worry about Windows, it seems it is failing across the board. I think we could merge this one, but explicitly stating that what you are doing isn't supported. Let's wait for @bluwy that said wanted to take a look

[bluwy]

Looks fine to me 👍 I thought this was touching the watcher ignores, which I'll have more reservations of if so, but since this is only for the entries, it looks like a good change for now.

[Jinjiang]

Thanks so much both.

So I guess we can merge this without the Windows CI pass? 🤔
Let me know anything I can help further.

[patak-dev]

There is an issue with playwright across the board, I'll merge this one without that test. Thanks for the patience @Jinjiang!