-
Notifications
You must be signed in to change notification settings - Fork 13
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
fix: entry state mutation in node compiler #550
base: main
Are you sure you want to change the base?
Conversation
Ok(self | ||
.0 | ||
.compiler | ||
.compile(&world, &mut CompileEnv::default()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call into the internal method directly, because we don't want to touch the driver wrapper with a default world.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we name the self.0.compiler
so to make it slightly more beautiful?
let c = &self.0.compiler;
c.compile(&world, &mut CompileEnv::default())
.mutate_entry(new_state) | ||
.map(Some) | ||
.map_err(map_node_error) | ||
Ok(self.universe.snapshot_with(Some(TaskInputs { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt that it's necessary for snapshot_with
to have Option<TaskInputs>
as its argument with all fields in TaskInputs
already Option<T>
.
Some(root) => { | ||
if let Ok(p) = root.strip_prefix(fp) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originallly, root.strip_prefix(fp)
had its order of operands reversed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may use world.entry_state().try_select_path_in_workspace(fp, true)
.
} | ||
|
||
pub fn compile_raw( | ||
&mut self, | ||
compile_by: CompileDocArgs, | ||
) -> napi::Result<NodeTypstCompileResult, NodeError> { | ||
let e = self.setup_compiler_by(compile_by)?; | ||
let world = self.setup_compiler_by(compile_by)?; | ||
|
||
let res = if self.0.universe().entry_state().is_inactive() { | ||
Err(map_node_error(error_once!("entry file is not set"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can directly return errors here, since no state is to recover:
- if let Some(entry_file) = e {
- self.0
- .universe_mut()
- .mutate_entry(entry_file)
- .map_err(map_node_error)?;
- }
Some(root) => { | ||
if let Ok(p) = root.strip_prefix(fp) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may use world.entry_state().try_select_path_in_workspace(fp, true)
.
The entry state handling in the ts-node-compiler is broken. This PR fixes the bug and do some other refactors:
setup_compiler_by
pure and build it on the basis of universe snapshot mechanismBoxedCompiler::compile_raw
andDynLayoutCompiler::vector
to the new signatureTest cases will come later.