Skip to content
This repository has been archived by the owner on Aug 15, 2024. It is now read-only.

Commit

Permalink
Sids incorrectly set for multiple factories call (#30)
Browse files Browse the repository at this point in the history
Fixes #11

we should visit CallExpr with factory BEFORE we wrap into
**withFactory**. This way, we do not need to add factory name to
**is_factory** hashmap, do not need this hashmap at all, and do not need
to remove factory name from **self.factory_map**.
  • Loading branch information
eaglus authored Mar 16, 2023
1 parent 24f7d9d commit 8d8d16b
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 28 deletions.
22 changes: 6 additions & 16 deletions src/effector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ impl<'a> State<'a> {
}
}

#[derive(Debug)]
#[derive(Debug, Clone)]
struct FactoryInfo {
imported_name: String,
}
Expand All @@ -580,7 +580,6 @@ pub struct Effector<'a, C: SourceMapper> {
candidate_name: Option<Ident>,
factory_paths: AHashSet<String>,
factory_map: AHashMap<Id, FactoryInfo>,
is_factory: AHashSet<Id>,
need_factory_import: bool,
factory_import_added: bool,
imports_to_add: AHashSet<ImportDecl>,
Expand All @@ -605,7 +604,6 @@ impl<'a, C: SourceMapper> Effector<'a, C> {
need_factory_import: false,
imports_to_add: AHashSet::new(),
with_factory_name: None,
is_factory: AHashSet::new(),
factory_import_added: false,
cm: Lrc::new(cm),
}
Expand Down Expand Up @@ -904,8 +902,6 @@ impl<'a, C: SourceMapper> VisitMut for Effector<'a, C> {
}

fn visit_mut_call_expr(&mut self, e: &mut CallExpr) {
let factories_used = !self.config.public.factories.is_empty();

if let Callee::Expr(expr) = &mut e.callee {
match &mut **expr {
Expr::Member(member) => {
Expand Down Expand Up @@ -978,9 +974,7 @@ impl<'a, C: SourceMapper> VisitMut for Effector<'a, C> {
);
}

if factories_used
&& !self.is_factory.contains(&ident.to_id())
&& self.factory_map.contains_key(&ident.to_id())
if let Some(FactoryInfo { imported_name }) = self.factory_map.get(&ident.to_id()).cloned()
{
let loc = self.cm.lookup_char_pos(ident.span.lo);

Expand All @@ -992,12 +986,6 @@ impl<'a, C: SourceMapper> VisitMut for Effector<'a, C> {
Some(self.add_import(quote_ident!("withFactory")));
}

let FactoryInfo { imported_name } = self
.factory_map
.remove(&ident.to_id())
.expect("Already checked for existence.");
self.is_factory.insert(ident.to_id());

let sid = generate_stable_id(
self.state.root.unwrap_or(""),
self.state.filename.unwrap_or(""),
Expand All @@ -1007,11 +995,14 @@ impl<'a, C: SourceMapper> VisitMut for Effector<'a, C> {
self.config.public.debug_sids,
);

let mut e_cloned = e.clone();
e_cloned.visit_mut_children_with(self);

let expr = swc_core::quote!(
"$factory({sid: $sid,fn:()=>$fun})" as Expr,
factory = self.with_factory_name.clone().unwrap(),
sid: Expr = sid.into(),
fun: Expr = Expr::Call(e.clone()),
fun: Expr = Expr::Call(e_cloned),
);

if let Expr::Call(mut call) = expr {
Expand Down Expand Up @@ -1047,7 +1038,6 @@ impl<'a, C: SourceMapper> VisitMut for Effector<'a, C> {
}
}
*e = call;
e.visit_mut_children_with(self);
return;
}
}
Expand Down
11 changes: 10 additions & 1 deletion tests/fixtures/factories/code.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
import createFactory from './factory';
import {createEvent} from "effector";
import { createFactory2 } from './factory';

const x = createFactory(createEvent());
const x = createFactory(createEvent());

const x1 = createFactory(123);
const x2 = createFactory(456, 789);
const nested1 = createFactory(11, createFactory(111));
const nested2 = createFactory(22, createFactory2(222));
const nested3 = createFactory(22, createFactory2(222, createFactory(333)));

createFactory("no-assign");
129 changes: 118 additions & 11 deletions tests/fixtures/factories/output.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,130 @@
import createFactory from './factory';
import { createEvent } from "effector";
import { createFactory2 } from './factory';
import { withFactory as _withFactory$0 } from "effector";
var _effectorFileName$0 = "/output.js";
const x = _withFactory$0({
sid: "18fxhw2x172ly",
sid: "a4ipirl4gkpj",
fn: ()=>createFactory(createEvent({
sid: "104pf2as7ai2y",
loc: {
file: _effectorFileName$0,
line: 4,
column: 24
},
name: "fn"
})),
name: "x",
sid: "29x8hthz296gt",
loc: {
file: _effectorFileName$0,
line: 5,
column: 24
},
name: "x"
})),
name: "name",
method: "default",
loc: {
file: _effectorFileName$0,
line: 4,
line: 5,
column: 10
}
});
const x1 = _withFactory$0({
sid: "34b7tq0at8fpy",
fn: ()=>createFactory(123),
name: "x1",
method: "default",
loc: {
file: _effectorFileName$0,
line: 7,
column: 11
}
});
const x2 = _withFactory$0({
sid: "1kozl3sawzyy9",
fn: ()=>createFactory(456, 789),
name: "x2",
method: "default",
loc: {
file: _effectorFileName$0,
line: 8,
column: 11
}
});
const nested1 = _withFactory$0({
sid: "1w17o98jwlkv3",
fn: ()=>createFactory(11, _withFactory$0({
sid: "2mhipt4tcjzu8",
fn: ()=>createFactory(111),
name: "nested1",
method: "default",
loc: {
file: _effectorFileName$0,
line: 9,
column: 34
}
})),
name: "nested1",
method: "default",
loc: {
file: _effectorFileName$0,
line: 9,
column: 16
}
});
const nested2 = _withFactory$0({
sid: "1foo588ty53em",
fn: ()=>createFactory(22, _withFactory$0({
sid: "1rtit3y9omj0f",
fn: ()=>createFactory2(222),
name: "nested2",
method: "createFactory2#1",
loc: {
file: _effectorFileName$0,
line: 10,
column: 34
}
})),
name: "nested2",
method: "default",
loc: {
file: _effectorFileName$0,
line: 10,
column: 16
}
});
const nested3 = _withFactory$0({
sid: "21r8czhd1vtif",
fn: ()=>createFactory(22, _withFactory$0({
sid: "28jbrqrp31rdk",
fn: ()=>createFactory2(222, _withFactory$0({
sid: "3mpga8xpw6gkz",
fn: ()=>createFactory(333),
name: "nested3",
method: "default",
loc: {
file: _effectorFileName$0,
line: 11,
column: 54
}
})),
name: "nested3",
method: "createFactory2#1",
loc: {
file: _effectorFileName$0,
line: 11,
column: 34
}
})),
name: "nested3",
method: "default",
loc: {
file: _effectorFileName$0,
line: 11,
column: 16
}
});
_withFactory$0({
sid: "1ymh502w4jx07",
fn: ()=>createFactory("no-assign"),
name: "nested3",
method: "default",
loc: {
file: _effectorFileName$0,
line: 13,
column: 0
}
});

0 comments on commit 8d8d16b

Please sign in to comment.