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

Yalc does NOT resolve dependencies of dependencies #95

Open
craigmiller160 opened this issue Aug 9, 2019 · 21 comments
Open

Yalc does NOT resolve dependencies of dependencies #95

craigmiller160 opened this issue Aug 9, 2019 · 21 comments

Comments

@craigmiller160
Copy link

craigmiller160 commented Aug 9, 2019

So there is another issue about this same thing, issue #16 , but it was closed without being resolved. I'm opening this one because this is a problem with an otherwise great tool and it needs to be fixed.

Let's say I have three project. A, B, and C. C is a dependency of B, so I "yalc add C" to B. This adds to package.json "C": "file:.yalc/C". That is a relative path from the project to the dependency in my local yalc folder.

Then I "yalc add B" to project A. It adds B in the local .yalc folder, but the package.json inside of it still just has that relative path to C. The problem is, C NEVER GETS COPIED. I know in #16 you keep saying it works for you, but I'm telling you I and the others who commented in that thread all absolutely disagree. The relative paths in the package.json break the ability to resolve the dependencies when you nest them.

The solution is simple: use absolute paths instead of relative ones. If C is added like this: "file://Users/Me/B/.yalc/C", then everything works.

It's a very simple change, to make it use absolute instead of relative paths. Even if you just made it an option, where someone could do yalc add with a flat, that would be enough.

I'm hoping this will be taken under advisement. If I get the time I'm tempted to create a branch, PR, or fork to add this feature. I work for a company with a lot of distant contractors and this would make their lives (and mine) easier.

Thanks.

PS. Apologies about not using better code formatting. I'm writing this on mobile.

@craigmiller160
Copy link
Author

I'm adding additional feedback here. This was bothering me, so I pulled down the source code for this tool and began poking around in it. Much to my surprise, I found that the code does seem to copy the .yalc directory along with the other content to each location it is deployed to. Then, after running it on my linux machine, I found that everything worked as expected.

Nevertheless, I'm having the previously mentioned problem on my work macbook. I am going to continue investigating this issue further, and I will report back with any findings. Maybe I can work out why the inconsistent results are occurring.

@wclr
Copy link
Owner

wclr commented Aug 10, 2019

It would be nice if you could create repro repo and provide a description of actions and expected results.

@craigmiller160
Copy link
Author

I will attempt to do so when I go into work on Monday. It is odd that I'm experiencing this issue on one device and not another when using your tool.

@dprogramming
Copy link

Just chiming in to let you know I'm having the same issue on Linux.

@craigmiller160
Copy link
Author

craigmiller160 commented Aug 14, 2019

Hey. I have pinpointed the issue, and I have reproduce-able steps for recreating it. I also have steps for solving it. After re-reading your README.md, this issue actually makes sense now. I'll recommend two possible solutions, and leave the choice of which one up to you @whitecolor.

THE PROBLEM IN DETAIL

The source of the problem was the initial "publish" command. This was what moved my local project into the ~/.yalc folder. Upon inspecting the copy of it in the ~/.yalc folder, I discovered that the internal .yalc directory was missing. So all the contents of my project were published EXCEPT the local .yalc dependencies. This was why the dependency-of-dependency thing wasn't working, because when I used "yalc add" to add this project to another one, the .yalc folder was already missing and wouldn't carry over.

THE SOLUTION

Upon reviewing the source code, I saw you were getting a list of all files using npm-packlist. I've never worked with that library, but it gave me the idea that it might be using the package.json to determine which files to publish or not. This matches well with your README.md, which says that only the files that would be published to npm get published.

So I added the .yalc directory to the "files" array of my package.json. The moment I did that, everything now worked. By explicitly including .yalc in the array of files to publish to npm, yalc now picks up that directory when runing "publish".

MORE INFORMATION

The default behavior of a project when it doesn't have a "files" section in the package.json is to publish everything, whereas the "files" section acts as a filter. This was why I didn't see this problem on my linux machine, the difference wasn't the OS, it was the fact that the project I tested there didn't have a files section. I have confirmed that adding such a section without adding .yalc to it causes the issue to occur on linux.

NEXT STEPS

As I see it, there are two possible next steps you could take.

  1. Update your README.md to explicitly call out this issue, and inform people that .yalc must be added to the "files" section of the package.json. Given that there are multiple threads that have been started about it in your issues already, I would consider this to be a major enough mistake that people (including myself) are making to warrant such direct, explicit attention.

  2. In addition to the npm-packlist request to get files, you can also do an explicit "fs" search for a .yalc directory, and add it no matter what. This would be my preferred solution, since .yalc is for local development and including it in the package.json "files" section seems to be polluting it somewhat.

Either way, I hope this helps folks (and you) dealing with this.

EDIT

I noticed a reference in your README to using .npmignore to ensure the .yalc directory gets included. I attempted to do this with no success. Only adding it to the "files" section of package.json works for me.

@wclr
Copy link
Owner

wclr commented Aug 14, 2019

npm-listfiles should list all files that npm pack uses to prepare the package.

If you use package.json's files list to explicitly define which folders/files should be included in the package, of course, you should include .yalc folder there.

If you don't use files (but for example use .npmignore to exclude certain content from the package) you don't need to do anything, .yalc the folder will be included.

I terms of which files will be included in the package yalc fully supposed to emulate standard behavior of npm client, because you may also want to publish the package to a remote repo, and in thie case your .yalc folder should be included too.

@craigmiller160
Copy link
Author

Yep, that makes sense. I would recommend adding a section to your README explicitly calling out this issue. As I said, there are several different threads on this same issue, highlighting it as a common gotcha (and explicitly mentioning the "files" section, since .npmignore did not work for me) would be my recommendation.

Regardless, at least this thread will exist for folks googling the issue.

@wclr
Copy link
Owner

wclr commented Aug 15, 2019

Ok I added some notice in docs.

@craigmiller160
Copy link
Author

Awesome. Thank you so much.

@dprogramming
Copy link

dprogramming commented Aug 16, 2019

Great, thanks @craigmiller160, this seems to be the solution!

For those reading this later I would like to add that, even if you don't have a files array in your package.json, keep in mind that npm also looks at the contents of your .gitignore file. So adding .yalc to your package.json files array might be necessary if you included .yalc in your .gitignore file as well.

@Zaynex
Copy link

Zaynex commented Jan 28, 2021

I have another problem.

I'm using lerna to manage packages.
For example, in @exmaplepkgs/pkg-a,
I have dependencies

 "@babylonjs/core": "^4.2.0",

then run,

cd @exmaplepkgs/pkg-a
yalc link

The exmaplepkgs/pkg-a dist file doesn't include @babylonjs/core code, just import it, like

import { Engine, Scene, Color4, MeshBuilder, Vector3, Matrix, MultiMaterial, StandardMaterial, Texture, SubMesh, HemisphericLight, Color3, ArcRotateCamera, Animation } from '@babylonjs/core';

Now, i use @exmaplepkgs/pkg-a link to project.

cd project
yalc link @exmaplepkgs/pkg-a

All things goes will, but in project, it shows errorCan't resolve '@babylonjs/core' in project/.yalc/@exmaplepkgs/pkg-a/dist

I have to copy @babylonjs/core node_modules to project/.yalc/@exmaplepkgs/pkg-a/node_modules

Anything I'm missing ?

@wclr
Copy link
Owner

wclr commented Jan 28, 2021

@Zaynex well yalc link flow probably not well/completely designed, because I don't use it. I use yalc add usually with workspaces, which allows to plug yalc'ed packages in a project. So in your case yalc folder is not linked to the project (in terms of package manager you use), you should either install deps there manually (you may use postyalc scripts for that - read in docs) or should make your PM include this package in it's working set, I didn't work with lerna not sure how the packages/folers are included. It would be nice if you report your final solution to the problem.

@MontolioV
Copy link

Hi @wclr ! Thank for such a great tool! It's wonderful helper!
From the discussion in the topic I understood that your intention was to have the same behavior as publish has. But in a current form it's prone to embed dependencies into an artifact that is published. That's against the idea of publish. To mitigate it we have to juggle the configs. Currently I don't see the way to both avoid polluting the published artifact with local dependencies and been able to work with chained dependencies conveniently.
What do you think about that? It seems like recursive update can install all needed local dependencies if we include the yalc.lock in the published files list. Then we can exclude .yalc safely.

@wclr
Copy link
Owner

wclr commented Mar 2, 2021

@MontolioV
You have to explain your case.

But in a current form it's prone to embed dependencies into an artifact that is published.

I don't get what you mean.

@MontolioV
Copy link

MontolioV commented Mar 3, 2021

TL;DR the best approach to not to accidentally pollute deployment package with dependencies of dependencies that I see now is to add yalc files to gitignore, then include yalc files in package.json files, and add prepublish hook yalc remove --all.

But in a current form it's prone to embed dependencies into an artifact that is published.

When using #95 (comment) solution, dependencies of dependencies will be included in deployment. As was mentioned it's not the best option. When we publish a package to a registry, we don't expect to publish some dependencies as well.

@wclr lets imagine we have 3 projects:

      pr2 
    /     \
pr1 ------ pr3

Project 2 depends on project 1, project 3 depends on project 1 and project 2.

  1. If pr2 depends on pr1@0.1.0 and pr3 depends on pr1@0.1.1 we may have the problem that was discussed in this issue, pr1@0.1.0 for pr2 will be missing and we should include yalc files in package.json files, or remove yalc files from gitignore, or do yalc update recursively (at least all these 3 approaches worked for me).
  2. If pr2 and pr3 both depends on pr1@0.1.1, and we installed dependencies (npm i/yarn), pr2 will be able to use pr1@0.1.1 from node_modules. That's much cleaner way, but it's only possible when we use pr1 in both projects.
    In this case we'll have to use the option 1:
pr1 --- pr2 --- pr3

If we exclude with .npmignore .yalc, but not yalc.lock, we can do yalc publish and then yalc update for a client project and for all it's yalc dependencies (if they have yalc.lock, dependencies of dependencies will be installed). I think it's then easier to do frequent deployments to remote registry. But it seems to be not so usual case, and client can achiev the same with a rather simple script.

@neaumusic
Copy link

neaumusic commented Apr 29, 2022

@MontolioV I'm wondering if prepublish would still delete the .yalc/ files before yalc publish, even though they're needed for yalc publish but not a regular one.

Does anyone have a definite way of permanently listing .yalc/ in files but ensuring that folder doesn't ever get published accidentally?

@Venryx
Copy link

Venryx commented Apr 29, 2022

Does anyone have a definite way of permanently listing .yalc/ in files but ensuring that folder doesn't ever get published accidentally?

@neaumusic You may be interested in the wrapper I created for just this use-case (if I'm understanding you correctly): #16 (comment)

@wclr
Copy link
Owner

wclr commented Apr 30, 2022

I'm wondering if prepublish would still delete the .yalc/ files before yalc publish, even though they're needed for yalc publish but not a regular one.

  1. You may use --no-scripts flag to avoid executing scripts while yalc publish.
  2. You may create scripts that will modify files entry and execute them in preyalcpublish/postyalcpublish, this is hacky.

@quantuminformation
Copy link

quantuminformation commented Dec 22, 2022

I'm trying to be cool to avoid by sharing tailwind configs with their plugins deps without having to depend on @tailwindcss/forms in both projects:

A = problem project, imports tailwind config from B (which needs @tailwindcss/forms)
B = our commons lib
C =

  "peerDependencies": {
    "@tailwindcss/forms": "^0.5.3"
  },

-> specified in B

im finding this too today, no peer dep (C) in B is downloaded by A (yalc add B)

In A, I don't want to also depend on C (@tailwindcss/forms) when it should be downloaded by B

@paulobmarcos
Copy link

Would this issue be solved with the portal: protocol instead of link:?

#213 (comment)

@gabrielschulhof
Copy link

yalc add already modifies a package's package.json to replace a dependency with a "file:.yalc/..." version. Why not also modify its "files" section to mention ".yalc"?

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

No branches or pull requests

10 participants