search

aurelia / webpack-plugin

A plugin for webpack that enables bundling Aurelia applications.
MIT License
90 stars 36 forks source link

Fixed relative path matching issue in ConventionDependenciesPlugin #194

Closed actuallyrob closed 2 years ago

actuallyrob commented 2 years ago

Fixed issue where minimatch would not match relative paths starting with ../ by feeding it resolved file paths. This caused issues in pnpm monorepos where Aurelia views of plugins were not included in the bundle because because they were wrongly disregarded.

actuallyrob commented 2 years ago

I assume I'd have to prove that the issues exist without my fix in place first?

To prove this I propose the following.

  1. Revert fix
  2. Add pnpm apps and let the CI pipeline fail
  3. Re-apply fix

Can you agree with these steps?

bigopon commented 2 years ago

Yes the steps look good, as long as we have a failing test at least.

actuallyrob commented 2 years ago

I am having issues with running the tests locally. I have created a start of a test app using the kendoui-bridge. I am trying to figure out how to run that app locally.

I am also questioning if the required dependencies can be installed without licensing issues. Do you think that could be an issue when using the aurelia-kendoui-bridge in this testcase?

bigopon commented 2 years ago

@actuallyrob I cannot answer the license question. Maybe we should ask kendo or the bridge repo owner?

For running test locally, what issue did you have and how did you run the test?

actuallyrob commented 2 years ago

I have found the reason this bug occurs in my code base. Somewhere in the process of converting my codebase to a pnpm monorepo, I had changed the node_modules directory that I pass to webpack of my application from the root of that package to the root of my monorepo. This worked fine since I have shamefully-hoist turned on in my pnpm config so all the node_module reside in the project root. This causes all modules to be resolved one folder up (../).

I no longer require a change in the aurelia-webpack-plugin. If you however still see a reason to apply my proposed fix, let me know. I know know the true cause of the issue, so I should be able to isolate it to create a test case.

bigopon commented 2 years ago

@actuallyrob glad to hear that you are able to resolved. For continuing this work, we can just wait until someone actually hits an issue with pnpm. Let's close this for now?

actuallyrob commented 2 years ago

Sounds good to me!