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

fix: hmr failed when add missing dep after watch started #845

Merged
merged 1 commit into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 23 additions & 19 deletions crates/mako/src/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,15 @@ enum ResolveError {
ResolveError { path: String, from: String },
}

#[derive(Debug, PartialEq)]
enum ResolverType {
#[derive(Debug, PartialEq, Eq, Hash)]
pub enum ResolverType {
Cjs,
Esm,
Css,
Ctxt,
}

pub struct Resolvers {
cjs: Resolver,
esm: Resolver,
css: Resolver,
ctxt: Resolver,
}
pub type Resolvers = HashMap<ResolverType, Resolver>;

#[derive(Debug, Clone)]
pub struct ExternalResource {
Expand Down Expand Up @@ -95,14 +90,15 @@ pub fn resolve(
mako_core::mako_profile_function!();
mako_core::mako_profile_scope!("resolve", &dep.source);
let resolver = if parse_path(&dep.source)?.has_query("context") {
&resolvers.ctxt
resolvers.get(&ResolverType::Ctxt)
} else if dep.resolve_type == ResolveType::Require {
&resolvers.cjs
resolvers.get(&ResolverType::Cjs)
} else if dep.resolve_type == ResolveType::Css {
&resolvers.css
resolvers.get(&ResolverType::Css)
} else {
&resolvers.esm
};
resolvers.get(&ResolverType::Esm)
}
.unwrap();

let source = dep.resolve_as.as_ref().unwrap_or(&dep.source);

Expand Down Expand Up @@ -297,12 +293,14 @@ pub fn get_resolvers(config: &Config) -> Resolvers {
let esm_resolver = get_resolver(config, ResolverType::Esm);
let css_resolver = get_resolver(config, ResolverType::Css);
let ctxt_resolver = get_resolver(config, ResolverType::Ctxt);
Resolvers {
cjs: cjs_resolver,
esm: esm_resolver,
css: css_resolver,
ctxt: ctxt_resolver,
}

let mut resolvers = HashMap::new();
resolvers.insert(ResolverType::Cjs, cjs_resolver);
resolvers.insert(ResolverType::Esm, esm_resolver);
resolvers.insert(ResolverType::Css, css_resolver);
resolvers.insert(ResolverType::Ctxt, ctxt_resolver);

resolvers
}

pub fn get_module_extensions() -> Vec<String> {
Expand Down Expand Up @@ -408,6 +406,12 @@ fn parse_alias(alias: HashMap<String, String>) -> Vec<(String, Vec<AliasMap>)> {
result
}

pub fn clear_resolver_cache(resolvers: &Resolvers) {
resolvers
.iter()
.for_each(|(_, resolver)| resolver.clear_entries());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里有更小颗粒度的 clear api 吗?

Copy link
Contributor Author

@zhangpanweb zhangpanweb Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没有更小颗粒的 api。本来是想给 nodejs_resolver pr 一个方法,单独删一个 entry,但是内部缓存用的 key 值是经过处理后的 path,不是传入的 request。看了下,hmr,只有在加文件的时候需要重新 resolve 添加的这个文件,所以删缓存理论上对后续hmr的速度影响也可控。给 nodejs_resolver 提了个 issue:web-infra-dev/nodejs_resolver#201 ,如果后续有方法,这里再优化一下

}

#[cfg(test)]
mod tests {
use std::collections::HashMap;
Expand Down
4 changes: 3 additions & 1 deletion crates/mako/src/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use mako_core::tracing::debug;
use crate::build::BuildError;
use crate::compiler::Compiler;
use crate::module::{Dependency, Module, ModuleId};
use crate::resolve;
use crate::resolve::{self, clear_resolver_cache};
use crate::task::{Task, TaskType};
use crate::transform_in_generate::transform_modules;
use crate::transformers::transform_virtual_css_modules::is_css_path;
Expand Down Expand Up @@ -111,6 +111,8 @@ impl Compiler {
// if found, add to modified queue
if has_added {
debug!("checking modules_with_missing_deps... since has added modules");
// clear resolver cache before resolving to avoid wrong result, i.e. add missing dep after watch started
clear_resolver_cache(&self.context.resolvers);
let mut modules_with_missing_deps =
self.context.modules_with_missing_deps.write().unwrap();
let mut module_graph = self.context.module_graph.write().unwrap();
Expand Down
51 changes: 43 additions & 8 deletions scripts/test-hmr.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ if (!fs.existsSync(tmp)) {
const port = 3000;
const DELAY_TIME = 500;

async function cleanup({ process, browser }) {
await killMakoDevServer();
await browser.close();
if (fs.existsSync(tmp)) {
fs.rmSync(tmp, { recursive: true });
}
async function cleanup({ process, browser } = {}) {
try {
await killMakoDevServer();
await browser?.close();
if (fs.existsSync(tmp)) {
fs.rmSync(tmp, { recursive: true });
}
} catch (e) {}
}

const tests = {};
Expand Down Expand Up @@ -987,6 +989,34 @@ export default A;
);
});

runTest('add missing dep after watch start', async () => {
await commonTest(
{
'/src/index.tsx': `
import React from 'react';
import ReactDOM from "react-dom/client";
import App from './App';
ReactDOM.createRoot(document.getElementById("root")!).render(<><App /><section>{Math.random()}</section></>);
`,
},
(lastResult) => {
assert.equal(lastResult.html, '', 'Initial render');
},
{
'/src/App.tsx': `
function App() {
return <div>App</div>;
}
export default App;
`,
},
(thisResult) => {
assert.equal(thisResult.html, '<div>App</div>', 'Second render');
},
true,
);
});

function normalizeFiles(files, makoConfig = {}) {
return {
'/public/index.html': `
Expand Down Expand Up @@ -1076,7 +1106,7 @@ function normalizeHtml(html) {
// e.g. <div>App<section>0.7551619733386135</section></div>
const re = /<section>(.+?)<\/section>/;
const match = html.match(re);
const random = match[1];
const random = match?.[1];
html = html.replace(re, '');
return { html, random };
}
Expand Down Expand Up @@ -1119,7 +1149,12 @@ async function commonTest(
console.log(`> ${chalk.green(name)}`);
await fn();
}
})().catch((e) => {
})().catch(async (e) => {
console.error(chalk.red(e));
await cleanup();
process.exit(1);
});

process.on('exit', async () => {
await cleanup();
});