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

feat: add resolve_id plugin hook #1625

Merged
merged 3 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
11 changes: 11 additions & 0 deletions crates/binding/src/js_hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,15 @@ pub struct JsHooks {
pub _on_generate_file: Option<JsFunction>,
#[napi(ts_type = "() => Promise<void>;")]
pub build_start: Option<JsFunction>,
#[napi(ts_type = "(source: string, importer: string) => Promise<{ id: string }>;")]
pub resolve_id: Option<JsFunction>,
}

pub struct TsFnHooks {
pub build_start: Option<ThreadsafeFunction<(), ()>>,
pub generate_end: Option<ThreadsafeFunction<Value, ()>>,
pub load: Option<ThreadsafeFunction<String, Option<LoadResult>>>,
pub resolve_id: Option<ThreadsafeFunction<(String, String), Option<ResolveIdResult>>>,
pub _on_generate_file: Option<ThreadsafeFunction<WriteFile, ()>>,
}

Expand All @@ -79,6 +82,9 @@ impl TsFnHooks {
load: hooks.load.as_ref().map(|hook| unsafe {
ThreadsafeFunction::from_napi_value(env.raw(), hook.raw()).unwrap()
}),
resolve_id: hooks.resolve_id.as_ref().map(|hook| unsafe {
ThreadsafeFunction::from_napi_value(env.raw(), hook.raw()).unwrap()
}),
_on_generate_file: hooks._on_generate_file.as_ref().map(|hook| unsafe {
ThreadsafeFunction::from_napi_value(env.raw(), hook.raw()).unwrap()
}),
Expand All @@ -99,3 +105,8 @@ pub struct LoadResult {
#[napi(js_name = "type")]
pub content_type: String,
}

#[napi(object, use_nullable = true)]
pub struct ResolveIdResult {
pub id: String,
}
27 changes: 26 additions & 1 deletion crates/binding/src/js_plugin.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::path::PathBuf;
use std::sync::Arc;

use crate::js_hook::{LoadResult, TsFnHooks, WriteFile};
use crate::js_hook::{LoadResult, ResolveIdResult, TsFnHooks, WriteFile};

pub struct JsPlugin {
pub hooks: TsFnHooks,
Expand All @@ -9,6 +10,7 @@ use anyhow::{anyhow, Result};
use mako::ast::file::{Content, JsContent};
use mako::compiler::Context;
use mako::plugin::{Plugin, PluginGenerateEndParams, PluginLoadParam};
use mako::resolve::{Resolution, ResolvedResource, ResolverResource};

impl Plugin for JsPlugin {
fn name(&self) -> &str {
Expand Down Expand Up @@ -47,6 +49,29 @@ impl Plugin for JsPlugin {
Ok(None)
}

fn resolve_id(
&self,
source: &str,
importer: &str,
_context: &Arc<Context>,
) -> Result<Option<ResolverResource>> {
if let Some(hook) = &self.hooks.resolve_id {
let x: Option<ResolveIdResult> =
hook.call((source.to_string(), importer.to_string()))?;
if let Some(x) = x {
return Ok(Some(ResolverResource::Resolved(ResolvedResource(
Resolution {
path: PathBuf::from(x.id),
query: None,
fragment: None,
package_json: None,
Copy link
Member

Choose a reason for hiding this comment

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

需要支持下 package json 的内容的传递,以保证需要使用报名的场景 or package.json 中 side effects

},
))));
}
}
Ok(None)
}

fn generate_end(&self, param: &PluginGenerateEndParams, _context: &Arc<Context>) -> Result<()> {
if let Some(hook) = &self.hooks.generate_end {
hook.call(serde_json::to_value(param)?)?
Expand Down
2 changes: 1 addition & 1 deletion crates/mako/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ mod module;
mod module_graph;
pub mod plugin;
mod plugins;
mod resolve;
pub mod resolve;
pub mod share;
pub mod stats;
pub mod utils;
Expand Down
26 changes: 24 additions & 2 deletions crates/mako/src/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@
Ok(None)
}

fn resolve_id(
&self,
_source: &str,
_importer: &str,
_context: &Arc<Context>,
) -> Result<Option<ResolverResource>> {
Ok(None)
}

fn next_build(&self, _next_build_param: &NextBuildParam) -> bool {
true
}
Expand Down Expand Up @@ -208,7 +217,6 @@
Ok(None)
}

#[allow(dead_code)]
pub fn transform_js(
&self,
param: &PluginTransformJsParam,
Expand All @@ -233,7 +241,6 @@
Ok(())
}

#[allow(dead_code)]
pub fn before_resolve(
&self,
param: &mut Vec<Dependency>,
Expand All @@ -245,6 +252,21 @@
Ok(())
}

pub fn resolve_id(
&self,
source: &str,
importer: &str,
context: &Arc<Context>,
) -> Result<Option<ResolverResource>> {
for plugin in &self.plugins {
let ret = plugin.resolve_id(source, importer, context)?;
if ret.is_some() {
return Ok(ret);

Check warning on line 264 in crates/mako/src/plugin.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/plugin.rs#L264

Added line #L264 was not covered by tests
}
}
Ok(None)
}

pub fn before_generate(&self, context: &Arc<Context>) -> Result<()> {
for plugin in &self.plugins {
plugin.generate_begin(context)?;
Expand Down
19 changes: 17 additions & 2 deletions crates/mako/src/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
use thiserror::Error;
use tracing::debug;

mod resolution;
mod resource;
pub(crate) use resource::{ExternalResource, ResolvedResource, ResolverResource};
pub use resolution::Resolution;
pub use resource::{ExternalResource, ResolvedResource, ResolverResource};

use crate::ast::file::parse_path;
use crate::compiler::Context;
Expand Down Expand Up @@ -49,6 +51,14 @@
crate::mako_profile_function!();
crate::mako_profile_scope!("resolve", &dep.source);

// plugin first
if let Some(resolved) = context
.plugin_driver
.resolve_id(&dep.source, path, context)?
{
return Ok(resolved);

Check warning on line 59 in crates/mako/src/resolve.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/resolve.rs#L59

Added line #L59 was not covered by tests
}

if dep.source.starts_with("virtual:") {
return Ok(ResolverResource::Virtual(PathBuf::from(&dep.source)));
}
Expand Down Expand Up @@ -244,7 +254,12 @@
// TODO: 临时方案,需要改成删除文件时删 resolve cache 里的内容
// 比如把 util.ts 改名为 util.tsx,目前应该是还有问题的
if resolution.path().exists() {
Ok(ResolverResource::Resolved(ResolvedResource(resolution)))
Ok(ResolverResource::Resolved(ResolvedResource(Resolution {
package_json: resolution.package_json().cloned(),
path: resolution.clone().into_path_buf(),
query: resolution.query().map(|q| q.to_string()),
fragment: resolution.fragment().map(|f| f.to_string()),
})))
} else {
Err(anyhow!(ResolveError {
path: source.to_string(),
Expand Down
63 changes: 63 additions & 0 deletions crates/mako/src/resolve/resolution.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
use std::fmt;
use std::path::{Path, PathBuf};
use std::sync::Arc;

use oxc_resolver::PackageJson;

#[derive(Clone)]
pub struct Resolution {
pub path: PathBuf,
pub query: Option<String>,
pub fragment: Option<String>,
pub package_json: Option<Arc<PackageJson>>,
}
Comment on lines +7 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

建议将结构体字段设为私有以增强封装性

当前,Resolution结构体的所有字段都声明为pub,这可能会暴露内部实现细节。建议将这些字段设为私有,并提供公有的访问方法(getter),以保持封装性和灵活性。


impl Resolution {
/// Returns the path without query and fragment
pub fn path(&self) -> &Path {
&self.path
}

Check warning on line 19 in crates/mako/src/resolve/resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/resolve/resolution.rs#L17-L19

Added lines #L17 - L19 were not covered by tests

/// Returns the path without query and fragment
pub fn into_path_buf(self) -> PathBuf {
self.path
}

Check warning on line 24 in crates/mako/src/resolve/resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/resolve/resolution.rs#L22-L24

Added lines #L22 - L24 were not covered by tests

/// Returns the path query `?query`, contains the leading `?`
pub fn query(&self) -> Option<&str> {
self.query.as_deref()
}

Check warning on line 29 in crates/mako/src/resolve/resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/resolve/resolution.rs#L27-L29

Added lines #L27 - L29 were not covered by tests

/// Returns the path fragment `#fragment`, contains the leading `#`
pub fn fragment(&self) -> Option<&str> {
self.fragment.as_deref()
}

Check warning on line 34 in crates/mako/src/resolve/resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/resolve/resolution.rs#L32-L34

Added lines #L32 - L34 were not covered by tests

/// Returns serialized package_json
pub fn package_json(&self) -> Option<&Arc<PackageJson>> {
self.package_json.as_ref()
}
Comment on lines +36 to +39
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

更新方法注释以反映实际返回值

package_json方法的注释为“返回序列化的package_json”,但实际上返回的是PackageJson的引用。为了避免误导,建议修改注释以准确描述方法的返回值。

修改建议:

- /// 返回序列化的package_json
+ /// 返回与路径关联的`PackageJson`引用(如果存在)
 pub fn package_json(&self) -> Option<&Arc<PackageJson>> {
     self.package_json.as_ref()
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Returns serialized package_json
pub fn package_json(&self) -> Option<&Arc<PackageJson>> {
self.package_json.as_ref()
}
/// 返回与路径关联的`PackageJson`引用(如果存在)
pub fn package_json(&self) -> Option<&Arc<PackageJson>> {
self.package_json.as_ref()
}


/// Returns the full path with query and fragment
pub fn full_path(&self) -> PathBuf {
let mut path = self.path.clone().into_os_string();
if let Some(query) = &self.query {
path.push(query);

Check warning on line 45 in crates/mako/src/resolve/resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/resolve/resolution.rs#L45

Added line #L45 was not covered by tests
}
if let Some(fragment) = &self.fragment {
path.push(fragment);

Check warning on line 48 in crates/mako/src/resolve/resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/resolve/resolution.rs#L48

Added line #L48 was not covered by tests
}
PathBuf::from(path)
}
}

impl fmt::Debug for Resolution {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Resolution")

Check warning on line 56 in crates/mako/src/resolve/resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/resolve/resolution.rs#L55-L56

Added lines #L55 - L56 were not covered by tests
.field("path", &self.path)
.field("query", &self.query)
.field("fragment", &self.fragment)
.field("package_json", &self.package_json.as_ref().map(|p| &p.path))

Check warning on line 60 in crates/mako/src/resolve/resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/resolve/resolution.rs#L58-L60

Added lines #L58 - L60 were not covered by tests
.finish()
}

Check warning on line 62 in crates/mako/src/resolve/resolution.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/resolve/resolution.rs#L62

Added line #L62 was not covered by tests
}
2 changes: 1 addition & 1 deletion crates/mako/src/resolve/resource.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::path::PathBuf;

use oxc_resolver::Resolution;
use crate::resolve::Resolution;

#[derive(Debug, Clone)]
pub struct ExternalResource {
Expand Down
3 changes: 3 additions & 0 deletions e2e/fixtures/plugins/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ assert(content.includes(`children: "foo.bar"`), `jsx in foo.bar works`);
assert(content.includes(`children: ".bar"`), `jsx in hoo.bar works`);
assert(content.includes(`children: ".haha"`), `plugin in node_modules works`);
assert(content.includes(`children: ".hoo"`), `relative plugin works`);

// resolve_id hook
assert(content.includes(`resolve_id mocked`), `resolve_id hook works`);
11 changes: 10 additions & 1 deletion e2e/fixtures/plugins/plugins.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,14 @@ module.exports = [
};
}
}
}
},
{
async resolveId(source, importer) {
if (source === 'resolve_id') {
console.log('resolveId', source, importer);
return { id: require('path').join(__dirname, 'resolve_id_mock.js') };
}
return null;
}
},
];
1 change: 1 addition & 0 deletions e2e/fixtures/plugins/resolve_id_mock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('resolve_id mocked');
1 change: 1 addition & 0 deletions e2e/fixtures/plugins/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ console.log(require('./foo.bar'));
console.log(require('./hoo.bar'));
console.log(require('./foo.haha'));
console.log(require('./foo.hoo'));
console.log(require('resolve_id'));
4 changes: 4 additions & 0 deletions packages/mako/binding.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export interface JsHooks {
}) => void;
onGenerateFile?: (path: string, content: Buffer) => Promise<void>;
buildStart?: () => Promise<void>;
resolveId?: (source: string, importer: string) => Promise<{ id: string }>;
}
export interface WriteFile {
path: string;
Expand All @@ -60,6 +61,9 @@ export interface LoadResult {
content: string;
type: string;
}
export interface ResolveIdResult {
id: string;
}
export interface BuildParams {
root: string;
config: {
Expand Down
Loading