Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

find location of packages outside of node_modules, support packages without package.json #576

Merged

Conversation

JeroenVinke
Copy link
Collaborator

fixes #567

@AStoker i'd like to hear your thoughts on the following. We need the root folder of a package (kept in description.location and looks something like c:/development/my-app/node_modules/my-plugin/) for two things:

  1. to look for (and read) the package.json of the package (which is used to determine the loader configuration)
  2. to determine what bundle to put files in. In other words, to prevent files from ending up in app-bundle.js when they should have ended up in vendor-bundle.js (or other)

Currently the root folder of packages inside the node_modules folder is determined as follows. You resolve the path of the package (as defined in aurelia.json) which could be c:/development/my-app/node_modules/my-plugin/dist/ and traverse up the folder tree until you hit the node_modules. The path until the first segment after node_modules is then used as the package root folder (c:/development/my-app/node_modules/my-plugin).

When you keep packages outside the node_modules folder, this trick does not work because there is no standard folder name like node_modules to look for in the path. So as far as I know there is no reliable way of knowing what the root folder of the package is when the package is not inside the node_modules folder.

So this PR enforces that if you define dependencies outside the node_modules folder, that you must define a packageRoot :

          {
            "name": "jquery",
            "path": "../external_modules/jquery/dist",
            "packageRoot": "../external_modules/jquery",
            "main": "jquery.min"
          }

@AStoker
Copy link
Contributor

AStoker commented Mar 31, 2017

I like that idea. I won't have time to really look into it until Monday though (gone over the weekend). So in your example, it would search up external_modules the same way it would search up node_modules?

@JeroenVinke
Copy link
Collaborator Author

It wouldn't traverse the directory tree to find the package root like it does with node_modules as that's not necessary when the user defines the packageRoot in aurelia.json

@AStoker
Copy link
Contributor

AStoker commented Apr 4, 2017

Gotcha, I'm tracking now. Sounds reasonable to me!

@JeroenVinke JeroenVinke force-pushed the fix/packages-outside-nodemodules branch from 41f0cbe to 3e46766 Compare April 7, 2017 15:53
@JeroenVinke JeroenVinke force-pushed the fix/packages-outside-nodemodules branch from 3e46766 to c225bb7 Compare April 8, 2017 19:11
@JeroenVinke JeroenVinke changed the title fix(package-analyzer): find location of packages outside of node_modules find location of packages outside of node_modules, support packages without package.json Apr 8, 2017
JeroenVinke added a commit to JeroenVinke/framework that referenced this pull request Apr 8, 2017
@JeroenVinke
Copy link
Collaborator Author

@EisenbergEffect I believe this is good to go.

I decided to add another fix to this PR, because they depend on each other. So this fixes:

When you merge this, could you also merge aurelia/framework#762?

@AStoker AStoker self-requested a review April 25, 2017 15:02
@AStoker
Copy link
Contributor

AStoker commented Apr 25, 2017

@EisenbergEffect, sorry to bump. Could we get this in?

@EisenbergEffect EisenbergEffect merged commit 8281a56 into aurelia:master Apr 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when bundling resources from outside node_modules
4 participants