From 5888bb902badfaf5baa659b8b399d55be78b4621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donny/=EA=B0=95=EB=8F=99=EC=9C=A4?= Date: Mon, 26 Aug 2024 18:25:58 +0900 Subject: [PATCH] fix: Allow guarded usage of node.js APIs from edge runtime (#69283) ### What? Improve the node.js linter ### Why? It should not warn for _safe_ usage. x-ref: https://vercel.slack.com/archives/C04DUD7EB1B/p1724522593235339 ### How? --- .../src/transforms/warn_for_edge_runtime.rs | 84 ++++++++++++++++++- .../edge-assert/guarded-nodejs/input.js | 9 ++ .../edge-assert/guarded-nodejs/output.js | 6 ++ .../edge-assert/guarded-process/input.js | 3 + .../edge-assert/guarded-process/output.js | 3 + 5 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 crates/next-custom-transforms/tests/fixture/edge-assert/guarded-nodejs/input.js create mode 100644 crates/next-custom-transforms/tests/fixture/edge-assert/guarded-nodejs/output.js create mode 100644 crates/next-custom-transforms/tests/fixture/edge-assert/guarded-process/input.js create mode 100644 crates/next-custom-transforms/tests/fixture/edge-assert/guarded-process/output.js diff --git a/crates/next-custom-transforms/src/transforms/warn_for_edge_runtime.rs b/crates/next-custom-transforms/src/transforms/warn_for_edge_runtime.rs index 7a4516060711d..5626f1de2f27c 100644 --- a/crates/next-custom-transforms/src/transforms/warn_for_edge_runtime.rs +++ b/crates/next-custom-transforms/src/transforms/warn_for_edge_runtime.rs @@ -1,10 +1,13 @@ use std::sync::Arc; +use rustc_hash::FxHashSet; use swc_core::{ + atoms::Atom, common::{errors::HANDLER, SourceMap, Span}, ecma::{ ast::{ - CallExpr, Callee, Expr, IdentName, ImportDecl, Lit, MemberExpr, MemberProp, NamedExport, + op, BinExpr, CallExpr, Callee, CondExpr, Expr, IdentName, IfStmt, ImportDecl, Lit, + MemberExpr, MemberProp, NamedExport, UnaryExpr, }, utils::{ExprCtx, ExprExt}, visit::{Visit, VisitWith}, @@ -20,6 +23,9 @@ pub fn warn_for_edge_runtime( cm, ctx, should_error_for_node_apis, + should_add_guards: false, + guarded_symbols: Default::default(), + guarded_process_props: Default::default(), } } @@ -27,6 +33,12 @@ struct WarnForEdgeRuntime { cm: Arc, ctx: ExprCtx, should_error_for_node_apis: bool, + + should_add_guards: bool, + /// We don't drop guards because a user may write a code like + /// `if(typeof clearImmediate !== "function") clearImmediate();` + guarded_symbols: FxHashSet, + guarded_process_props: FxHashSet, } const EDGE_UNSUPPORTED_NODE_APIS: &[&str] = &[ @@ -142,6 +154,14 @@ Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime", } fn emit_unsupported_api_error(&self, span: Span, api_name: &str) -> Option<()> { + if self + .guarded_symbols + .iter() + .any(|guarded| guarded == api_name) + { + return None; + } + let loc = self.cm.lookup_line(span.lo).ok()?; let msg = format!( @@ -170,9 +190,48 @@ Learn more: https://nextjs.org/docs/api-reference/edge-runtime", if !self.is_in_middleware_layer() || prop.sym == "env" { return; } + if self.guarded_process_props.contains(&prop.sym) { + return; + } self.emit_unsupported_api_error(span, &format!("process.{}", prop.sym)); } + + fn add_guards(&mut self, test: &Expr) { + let old = self.should_add_guards; + self.should_add_guards = true; + test.visit_children_with(self); + self.should_add_guards = old; + } + + fn add_guard_for_test(&mut self, test: &Expr) { + if !self.should_add_guards { + return; + } + + match test { + Expr::Ident(ident) => { + self.guarded_symbols.insert(ident.sym.clone()); + } + Expr::Member(member) => { + if member.obj.is_global_ref_to(&self.ctx, "process") { + if let MemberProp::Ident(prop) = &member.prop { + self.guarded_process_props.insert(prop.sym.clone()); + } + } + } + Expr::Bin(BinExpr { + left, + right, + op: op!("===") | op!("==") | op!("!==") | op!("!="), + .. + }) => { + self.add_guard_for_test(left); + self.add_guard_for_test(right); + } + _ => (), + } + } } impl Visit for WarnForEdgeRuntime { @@ -186,6 +245,13 @@ impl Visit for WarnForEdgeRuntime { } } + fn visit_cond_expr(&mut self, node: &CondExpr) { + self.add_guards(&node.test); + + node.cons.visit_with(self); + node.alt.visit_with(self); + } + fn visit_expr(&mut self, n: &Expr) { if let Expr::Ident(ident) = n { if ident.ctxt == self.ctx.unresolved_ctxt { @@ -201,6 +267,13 @@ impl Visit for WarnForEdgeRuntime { n.visit_children_with(self); } + fn visit_if_stmt(&mut self, node: &IfStmt) { + self.add_guards(&node.test); + + node.cons.visit_with(self); + node.alt.visit_with(self); + } + fn visit_import_decl(&mut self, n: &ImportDecl) { n.visit_children_with(self); @@ -225,4 +298,13 @@ impl Visit for WarnForEdgeRuntime { self.warn_if_nodejs_module(n.span, &module_specifier.value); } } + + fn visit_unary_expr(&mut self, node: &UnaryExpr) { + if node.op == op!("typeof") { + self.add_guard_for_test(&node.arg); + return; + } + + node.visit_children_with(self); + } } diff --git a/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-nodejs/input.js b/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-nodejs/input.js new file mode 100644 index 0000000000000..d0d34957ae4b5 --- /dev/null +++ b/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-nodejs/input.js @@ -0,0 +1,9 @@ +if (typeof clearImmediate === 'function') { + console.log(clearImmediate()) +} + +// We allow this. +const scheduleTimeoutA = + typeof setImmediate === 'function' ? setImmediate : setTimeout +const scheduleTimeoutB = + typeof setImmediate !== 'function' ? setTimeout : setImmediate diff --git a/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-nodejs/output.js b/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-nodejs/output.js new file mode 100644 index 0000000000000..4ce35310d406d --- /dev/null +++ b/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-nodejs/output.js @@ -0,0 +1,6 @@ +if (typeof clearImmediate === 'function') { + console.log(clearImmediate()); +} +// We allow this. +const scheduleTimeoutA = typeof setImmediate === 'function' ? setImmediate : setTimeout; +const scheduleTimeoutB = typeof setImmediate !== 'function' ? setTimeout : setImmediate; diff --git a/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-process/input.js b/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-process/input.js new file mode 100644 index 0000000000000..6ca9667a2f2db --- /dev/null +++ b/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-process/input.js @@ -0,0 +1,3 @@ +if (typeof process.loadEnvFile === 'function') { + console.log(process.loadEnvFile()) +} diff --git a/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-process/output.js b/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-process/output.js new file mode 100644 index 0000000000000..536ed876cf32e --- /dev/null +++ b/crates/next-custom-transforms/tests/fixture/edge-assert/guarded-process/output.js @@ -0,0 +1,3 @@ +if (typeof process.loadEnvFile === 'function') { + console.log(process.loadEnvFile()); +}