-
Notifications
You must be signed in to change notification settings - Fork 469
Detect package name mismatch #7604
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
Detect package name mismatch #7604
Conversation
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
@nojaf Can you add a CHANGELOG entry? |
I'm not sure if this fix is a good idea. Testing it in the test repository is not feasible because having a different name in package.json results in a different folder during the Yarn installation. I believe other package managers might face similar issues, as the package name is usually aligned with the folder name. Therefore, I think it would be best to resolve this problem by aligning the names. It seems reasonable to accept this as a limitation, especially in the case of monorepos where I don't believe there's a workaround. |
This is actually already part of the docs: https://rescript-lang.org/docs/manual/v11.0.0/build-configuration#name-namespace Should we convert this PR into a warning-only thing? |
Ok, yes, warning only is fine with me! |
|
Hi @jfrolich, I found the problem for #7493
It turns out that my
package.json
name key was not exactly the same as what was used in myrescript.json
setup. Causing a mismatch incompile.rs
.So, my setup is:
project package.json
So, this gets installed as
node_modules/@nojaf/rescript-firebase
But
node_modules/@nojaf/rescript-firebase/package.json
had"name": "rescript-firebase"
.While
node_modules/@nojaf/rescript-firebase/rescript.json
had"name": "@nojaf/rescript-firebase"
.I would be okay with updating my own code. But this does seem like a restriction the previous build system didn't have. So, maybe updating the code (using
config.name.clone(),
) makes sense.I'm not sure.