X Tutup
The Wayback Machine - https://web.archive.org/web/20201104074734/https://github.com/lerna/lerna/issues/2064
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

Root lifecycle being skipped with missing root project manifest name #2064

Open
bigtimebuddy opened this issue May 1, 2019 · 4 comments
Open

Comments

@bigtimebuddy
Copy link
Contributor

@bigtimebuddy bigtimebuddy commented May 1, 2019

I ran into a use-case and I'm not sure if this is a bug or feature. Either way maybe some documentation might alleviate the confusion. Was definitely a gotcha that kept me busy for a few hours tracking it down.

For reference: pixijs/pixi.js#5635

I think this might be a regression in behavior, but I'm not 100% positive about that. Maybe I just wasn't paying close enough attention 🤷‍♂

Expected Behavior

When doing a publish or version command when there's a missing name field in the root-level package.json file and where . is not a packages entry in the lerna.json file, then lifecycle hooks (e.g. postversion, prepublishOnly, etc) should get invoked at the root-level.

Current Behavior

Because the default name of the project folder can match the name of a package being published, then Lerna assumes that hasRootedLeaf is true and lifecycle methods are skipped.

Possible Solution

The workaround that I found was to give the root package.json a unique name not shared by any packages.

One solution would be to explicitly require and break on a missing name field in the root-level package.json.

Another idea: I'm not sure that using npm's default behavior in setting basename for the root project.manifest is the best choice here. Maybe it would be more appropriate to assign it a unique name, to avoid the case that the folder name happens to overlap an internal package name. Fewer chances of conflicts and accidentally skipping lifecycle hooks.

Steps to Reproduce (for bugs)

  1. Create a standard lerna project (no root leaf package) and name any package the same thing as the root repo project folder's name.
  2. Remove any name field from the root-level
  3. Also, add a preversion script at the root-level package.json
  4. Run lerna version or lerna publish
  5. Notice that preversion is never called

Context

Recently published a new major version of PixiJS and lifecycle method that was assumed to be hit, was never. This created a slightly botched published which we hope to correct ASAP.

Your Environment

Executable Version
lerna --version 3.13.3
npm --version 6.9.0
node --version 10.15.3
OS Version
NAME VERSION
macOS Mojave 10.14.4
@evocateur
Copy link
Member

@evocateur evocateur commented May 3, 2019

Yikes, sorry about that. I'm cool with throwing an error if the project manifest (package.json that is a sibling of lerna.json, usually in the repo root) doesn't have a name field, a clear error message makes it an easy fix. We could also certainly make the fallback name more robust, like "HAY_FOLKS_GIMME_A_NAME_${Date.now()}" or something...

@sturdynut
Copy link

@sturdynut sturdynut commented Oct 22, 2019

Can I help with this?

@evocateur
Copy link
Member

@evocateur evocateur commented Oct 22, 2019

@sturdynut Sure!

@sturdynut
Copy link

@sturdynut sturdynut commented Nov 15, 2019

Apologies for the delay, I've been a bit behind on my open source work. I got this pulled down and was going to implement what you suggested @evocateur; to throw a ValidationError if the root package.json does not have a name or the name matches the name of one of the packages. Does that sound correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.
X Tutup