Skip to content

Commit

Permalink
🐛 - Fix rename bug inside of a namespace (#117)
Browse files Browse the repository at this point in the history
  • Loading branch information
jfrolich authored May 24, 2024
1 parent d1efa2f commit f406559
Show file tree
Hide file tree
Showing 16 changed files with 106 additions and 25 deletions.
5 changes: 3 additions & 2 deletions src/build/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,11 @@ pub fn cleanup_previous_build(
.map(|module_name| {
// if the module is a namespace, we need to mark the whole namespace as dirty when a module has been deleted
if let Some(namespace) = helpers::get_namespace_from_module_name(module_name) {
return namespace;
return vec![namespace, module_name.to_string()];
}
module_name.to_string()
vec![module_name.to_string()]
})
.flatten()
.collect::<AHashSet<String>>();

build_state.deleted_modules = deleted_module_names;
Expand Down
3 changes: 2 additions & 1 deletion src/build/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ fn get_dep_modules(
_ => dep_first,
};
let namespaced_name = dep.to_owned() + "-" + namespace;
if package_modules.contains(&namespaced_name) {
if package_modules.contains(&namespaced_name) || valid_modules.contains(&namespaced_name)
{
namespaced_name
} else {
dep.to_string()
Expand Down
9 changes: 9 additions & 0 deletions testrepo/packages/main/src/InternalDep.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Generated by ReScript, PLEASE EDIT WITH CARE


var value = 1;

export {
value ,
}
/* No side effect */
1 change: 1 addition & 0 deletions testrepo/packages/main/src/InternalDep.res
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let value = 1
3 changes: 3 additions & 0 deletions testrepo/packages/main/src/Main.mjs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// Generated by ReScript, PLEASE EDIT WITH CARE

import * as Dep01 from "@testrepo/dep01/src/Dep01.mjs";
import * as InternalDep from "./InternalDep.mjs";

console.log("01");

Dep01.log();

console.log(InternalDep.value);

var $$Array;

var $$String;
Expand Down
2 changes: 2 additions & 0 deletions testrepo/packages/main/src/Main.res
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
Js.log("01")
Dep01.log()

Js.log(InternalDep.value)

module Array = Belt.Array
module String = Js.String
1 change: 1 addition & 0 deletions testrepo/packages/main/src/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ Hello world
02
a
b
1
37 changes: 26 additions & 11 deletions tests/compile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,24 @@ bold "Test: It should compile"
if rewatch clean &> /dev/null;
then
success "Repo Cleaned"
else
else
error "Error Cleaning Repo"
exit 1
fi

if rewatch &> /dev/null;
if rewatch &> /dev/null;
then
success "Repo Built"
else
else
error "Error Building Repo"
exit 1
fi


if git diff --exit-code ./;
if git diff --exit-code ./;
then
success "Testrepo has no changes"
else
else
error "Build has changed"
exit 1
fi
Expand All @@ -35,6 +35,21 @@ node ./packages/main/src/Main.mjs > ./packages/main/src/output.txt
mv ./packages/main/src/Main.res ./packages/main/src/Main2.res
rewatch build --no-timing=true &> ../tests/snapshots/rename-file.txt
mv ./packages/main/src/Main2.res ./packages/main/src/Main.res

# Rename a file with a dependent - this should trigger an error
mv ./packages/main/src/InternalDep.res ./packages/main/src/InternalDep2.res
rewatch build --no-timing=true &> ../tests/snapshots/rename-file-internal-dep.txt
# replace the absolute path so the snapshot is the same on all machines
replace "s/$(pwd | sed "s/\//\\\\\//g")//g" ../tests/snapshots/rename-file-internal-dep.txt
mv ./packages/main/src/InternalDep2.res ./packages/main/src/InternalDep.res

# Rename a file with a dependent in a namespaced package - this should trigger an error (regression)
mv ./packages/new-namespace/src/Other_module.res ./packages/new-namespace/src/Other_module2.res
rewatch build --no-timing=true &> ../tests/snapshots/rename-file-internal-dep-namespace.txt
# replace the absolute path so the snapshot is the same on all machines
replace "s/$(pwd | sed "s/\//\\\\\//g")//g" ../tests/snapshots/rename-file-internal-dep-namespace.txt
mv ./packages/new-namespace/src/Other_module2.res ./packages/new-namespace/src/Other_module.res

rewatch build &> /dev/null
mv ./packages/main/src/ModuleWithInterface.resi ./packages/main/src/ModuleWithInterface2.resi
rewatch build --no-timing=true &> ../tests/snapshots/rename-interface-file.txt
Expand Down Expand Up @@ -66,10 +81,10 @@ git checkout -- packages/new-namespace/src/NS_alias.res
rewatch build &> /dev/null

# make sure we don't have changes in the test repo
if git diff --exit-code ./;
if git diff --exit-code ./;
then
success "Output is correct"
else
else
error "Output is incorrect"
exit 1
fi
Expand All @@ -81,18 +96,18 @@ new_files=$(git ls-files --others --exclude-standard ./)
if [[ $new_files = "" ]];
then
success "No new files created"
else
else
error "❌ - New files created"
printf "${new_files}\n"
exit 1
fi

# see if the snapshots have changed
changed_snapshots=$(git ls-files --modified ../tests/snapshots)
if git diff --exit-code ../tests/snapshots &> /dev/null;
if git diff --exit-code ../tests/snapshots &> /dev/null;
then
success "Snapshots are correct"
else
else
error "Snapshots are incorrect:"
# print filenames in the snapshot dir call bold with the filename
# and then cat their contents
Expand All @@ -105,4 +120,4 @@ else
done

exit 1
fi
fi
2 changes: 1 addition & 1 deletion tests/snapshots/dependency-cycle.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[1/7]📦 Building package tree...[1/7] 📦 Built package tree in 0.00s
[2/7] 🕵️ Finding source files...[2/7] 🕵️ Found source files in 0.00s
[3/7] 📝 Reading compile state...[3/7] 📝 Read compile state 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 0/10 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 0/11 0.00s
[5/7] 🧱 Parsed 1 source files in 0.00s
[6/7] ️🌴 Collected deps in 0.00s
[7/7] ️🛑 Compiled 0 modules in 0.00s
Expand Down
2 changes: 1 addition & 1 deletion tests/snapshots/remove-file.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[1/7]📦 Building package tree...[1/7] 📦 Built package tree in 0.00s
[2/7] 🕵️ Finding source files...[2/7] 🕵️ Found source files in 0.00s
[3/7] 📝 Reading compile state...[3/7] 📝 Read compile state 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 1/10 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 1/11 0.00s
[5/7] 🧱 Parsed 0 source files in 0.00s
[6/7] ️🌴 Collected deps in 0.00s
[7/7] ️🛑 Compiled 1 modules in 0.00s
Expand Down
24 changes: 24 additions & 0 deletions tests/snapshots/rename-file-internal-dep-namespace.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[1/7]📦 Building package tree...[1/7] 📦 Built package tree in 0.00s
[2/7] 🕵️ Finding source files...[2/7] 🕵️ Found source files in 0.00s
[3/7] 📝 Reading compile state...[3/7] 📝 Read compile state 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 2/11 0.00s
[5/7] 🧱 Parsed 2 source files in 0.00s
[6/7] ️🌴 Collected deps in 0.00s
[7/7] ️🛑 Compiled 3 modules in 0.00s

We've found a bug for you!
/packages/new-namespace/src/NS_alias.res:2:1-16

1 │ let hello_world = () => "Hello world"
2 │ Other_module.bla()
3 │

The module or file Other_module can't be found.
- If it's a third-party dependency:
- Did you add it to the "bs-dependencies" or "bs-dev-dependencies" in bsconfig.json?
- Did you include the file's directory to the "sources" in bsconfig.json?


Hint: Did you mean Other_module2?

Error Building:  ️🛑 Error Running Incremental Build:  ️🛑 Failed to Compile. See Errors Above
Expand Down
24 changes: 24 additions & 0 deletions tests/snapshots/rename-file-internal-dep.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[1/7]📦 Building package tree...[1/7] 📦 Built package tree in 0.00s
[2/7] 🕵️ Finding source files...[2/7] 🕵️ Found source files in 0.00s
[3/7] 📝 Reading compile state...[3/7] 📝 Read compile state 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 2/11 0.00s
[5/7] 🧱 Parsed 2 source files in 0.00s
[6/7] ️🌴 Collected deps in 0.00s
[7/7] ️🛑 Compiled 2 modules in 0.00s

We've found a bug for you!
/packages/main/src/Main.res:4:8-24

2 │ Dep01.log()
3 │
4 │ Js.log(InternalDep.value)
5 │
6 │ module Array = Belt.Array

The module or file InternalDep can't be found.
- If it's a third-party dependency:
- Did you add it to the "bs-dependencies" or "bs-dev-dependencies" in bsconfig.json?
- Did you include the file's directory to the "sources" in bsconfig.json?


Error Building:  ️🛑 Error Running Incremental Build:  ️🛑 Failed to Compile. See Errors Above
Expand Down
2 changes: 1 addition & 1 deletion tests/snapshots/rename-file-with-interface.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
[2/7] 🕵️ Finding source files...Warning: No implementation file found for interface file (skipping): src/ModuleWithInterface.resi
[2/7] 🕵️ Found source files in 0.00s
[3/7] 📝 Reading compile state...[3/7] 📝 Read compile state 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 2/10 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 2/11 0.00s
[5/7] 🧱 Parsed 1 source files in 0.00s
[6/7] ️🌴 Collected deps in 0.00s
[7/7] 🤺 ️Compiled 1 modules in 0.00s
Expand Down
2 changes: 1 addition & 1 deletion tests/snapshots/rename-file.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[1/7]📦 Building package tree...[1/7] 📦 Built package tree in 0.00s
[2/7] 🕵️ Finding source files...[2/7] 🕵️ Found source files in 0.00s
[3/7] 📝 Reading compile state...[3/7] 📝 Read compile state 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 1/10 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 1/11 0.00s
[5/7] 🧱 Parsed 1 source files in 0.00s
[6/7] ️🌴 Collected deps in 0.00s
[7/7] 🤺 ️Compiled 1 modules in 0.00s
Expand Down
2 changes: 1 addition & 1 deletion tests/snapshots/rename-interface-file.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
[2/7] 🕵️ Finding source files...Warning: No implementation file found for interface file (skipping): src/ModuleWithInterface2.resi
[2/7] 🕵️ Found source files in 0.00s
[3/7] 📝 Reading compile state...[3/7] 📝 Read compile state 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 1/10 0.00s
[4/7] 🧹 Cleaning up previous build...[4/7] 🧹 Cleaned 1/11 0.00s
[5/7] 🧱 Parsed 1 source files in 0.00s
[6/7] ️🌴 Collected deps in 0.00s
[7/7] 🤺 ️Compiled 1 modules in 0.00s
Expand Down
12 changes: 6 additions & 6 deletions tests/suffix.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ sleep 1
if rewatch clean &> /dev/null;
then
success "Repo Cleaned"
else
else
error "Error Cleaning Repo"
exit 1
fi
Expand All @@ -19,26 +19,26 @@ replace "s/.mjs/.res.js/g" bsconfig.json
if rewatch build &> /dev/null;
then
success "Repo Built"
else
else
error "Error building repo"
exit 1
fi

# Count files with new extension
file_count=$(find . -name *.res.js | wc -l)

if [ "$file_count" -eq 9 ];
if [ "$file_count" -eq 10 ];
then
success "Found files with correct suffix"
else
else
error "Suffix not correctly used"
exit 1
fi

if rewatch clean &> /dev/null;
then
success "Repo Cleaned"
else
else
error "Error Cleaning Repo"
exit 1
fi
Expand All @@ -50,7 +50,7 @@ replace "s/.res.js/.mjs/g" bsconfig.json
if rewatch build &> /dev/null;
then
success "Repo Built"
else
else
error "Error building repo"
exit 1
fi

0 comments on commit f406559

Please sign in to comment.