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.

Sign up for GitHub

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

Mark import as external #880

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Mark import as external #880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
71 changes: 53 additions & 18 deletions napi/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ pub fn transform_style_attribute(ctx: CallContext) -> napi::Result<JsUnknown> {
mod bundle {
use super::*;
use crossbeam_channel::{self, Receiver, Sender};
use lightningcss::bundler::FileProvider;
use napi::{Env, JsFunction, JsString, NapiRaw};
use lightningcss::bundler::{FileProvider, ResolveResult};
use napi::{Env, JsBoolean, JsFunction, JsString, NapiRaw};
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::sync::Mutex;
Expand Down Expand Up @@ -166,9 +166,33 @@ mod bundle {
unsafe impl Sync for JsSourceProvider {}
unsafe impl Send for JsSourceProvider {}

enum StringOrBool {
String(String),
Bool(bool),
}

impl TryFrom<JsUnknown> for StringOrBool {
type Error = napi::Error;

fn try_from(value: JsUnknown) -> Result<Self, Self::Error> {
let ty: napi::ValueType = value.get_type()?;
match ty {
napi::ValueType::String => Ok(StringOrBool::String(
JsString::try_from(value)?.into_utf8()?.into_owned()?,
)),
napi::ValueType::Boolean => Ok(StringOrBool::Bool(JsBoolean::try_from(value)?.get_value()?)),
_ => Err(napi::Error::new(
napi::Status::InvalidArg,
format!("expect string or boolean, got: {}", ty),
)),
}
}
}

// Allocate a single channel per thread to communicate with the JS thread.
thread_local! {
static CHANNEL: (Sender<napi::Result<String>>, Receiver<napi::Result<String>>) = crossbeam_channel::unbounded();
static RESOLVER_CHANNEL: (Sender<napi::Result<StringOrBool>>, Receiver<napi::Result<StringOrBool>>) = crossbeam_channel::unbounded();
}

impl SourceProvider for JsSourceProvider {
Expand Down Expand Up @@ -203,9 +227,9 @@ mod bundle {
}
}

fn resolve(&self, specifier: &str, originating_file: &Path) -> Result<PathBuf, Self::Error> {
fn resolve(&self, specifier: &str, originating_file: &Path) -> Result<ResolveResult, Self::Error> {
if let Some(resolve) = &self.resolve {
return CHANNEL.with(|channel| {
return RESOLVER_CHANNEL.with(|channel| {
let message = ResolveMessage {
specifier: specifier.to_owned(),
originating_file: originating_file.to_str().unwrap().to_owned(),
Expand All @@ -215,20 +239,28 @@ mod bundle {
resolve.call(message, ThreadsafeFunctionCallMode::Blocking);
let result = channel.1.recv().unwrap();
match result {
Ok(result) => Ok(PathBuf::from_str(&result).unwrap()),
Ok(StringOrBool::String(file)) => Ok(ResolveResult::File(PathBuf::from_str(&file).unwrap())),
Ok(StringOrBool::Bool(true)) => Ok(ResolveResult::External(specifier.to_owned())),
Ok(StringOrBool::Bool(false)) => Err(napi::Error::new(
napi::Status::InvalidArg,
format!(
"Invalid value `false` returned from `resolve` callback for `{}`",
specifier
),
)),
Err(e) => Err(e),
}
});
}

Ok(originating_file.with_file_name(specifier))
Ok(originating_file.with_file_name(specifier).into())
}
}

struct ResolveMessage {
specifier: String,
originating_file: String,
tx: Sender<napi::Result<String>>,
tx: Sender<napi::Result<StringOrBool>>,
}

struct ReadMessage {
Expand All @@ -241,17 +273,20 @@ mod bundle {
tx: Sender<napi::Result<String>>,
}

fn await_promise(env: Env, result: JsUnknown, tx: Sender<napi::Result<String>>) -> napi::Result<()> {
fn await_promise<T, Cb>(env: Env, result: JsUnknown, tx: Sender<napi::Result<T>>, parse: Cb) -> napi::Result<()>
where
T: 'static,
Cb: 'static + Fn(JsUnknown) -> Result<T, napi::Error>,
{
// If the result is a promise, wait for it to resolve, and send the result to the channel.
// Otherwise, send the result immediately.
if result.is_promise()? {
let result: JsObject = result.try_into()?;
let then: JsFunction = get_named_property(&result, "then")?;
let tx2 = tx.clone();
let cb = env.create_function_from_closure("callback", move |ctx| {
let res = ctx.get::<JsString>(0)?.into_utf8()?;
let s = res.into_owned()?;
tx.send(Ok(s)).unwrap();
let res = parse(ctx.get::<JsUnknown>(0)?)?;
tx.send(Ok(res)).unwrap();
ctx.env.get_undefined()
})?;
let eb = env.create_function_from_closure("error_callback", move |ctx| {
Expand All @@ -261,10 +296,8 @@ mod bundle {
})?;
then.call(Some(&result), &[cb, eb])?;
} else {
let result: JsString = result.try_into()?;
let utf8 = result.into_utf8()?;
let s = utf8.into_owned()?;
tx.send(Ok(s)).unwrap();
let result = parse(result)?;
tx.send(Ok(result)).unwrap();
}

Ok(())
Expand All @@ -274,10 +307,10 @@ mod bundle {
let specifier = ctx.env.create_string(&ctx.value.specifier)?;
let originating_file = ctx.env.create_string(&ctx.value.originating_file)?;
let result = ctx.callback.unwrap().call(None, &[specifier, originating_file])?;
await_promise(ctx.env, result, ctx.value.tx)
await_promise(ctx.env, result, ctx.value.tx, |unknown| unknown.try_into())
}

fn handle_error(tx: Sender<napi::Result<String>>, res: napi::Result<()>) -> napi::Result<()> {
fn handle_error<T>(tx: Sender<napi::Result<T>>, res: napi::Result<()>) -> napi::Result<()> {
match res {
Ok(_) => Ok(()),
Err(e) => {
Expand All @@ -295,7 +328,9 @@ mod bundle {
fn read_on_js_thread(ctx: ThreadSafeCallContext<ReadMessage>) -> napi::Result<()> {
let file = ctx.env.create_string(&ctx.value.file)?;
let result = ctx.callback.unwrap().call(None, &[file])?;
await_promise(ctx.env, result, ctx.value.tx)
await_promise(ctx.env, result, ctx.value.tx, |unknown| {
JsString::try_from(unknown)?.into_utf8()?.into_owned()
})
}

fn read_on_js_thread_wrapper(ctx: ThreadSafeCallContext<ReadMessage>) -> napi::Result<()> {
Expand Down
27 changes: 26 additions & 1 deletion node/test/bundle.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ test('resolve return non-string', async () => {
}

if (!error) throw new Error(`\`testResolveReturnNonString()\` failed. Expected \`bundleAsync()\` to throw, but it did not.`);
assert.equal(error.message, 'expect String, got: Number');
assert.equal(error.message, 'expect string or boolean, got: Number');
assert.equal(error.fileName, 'tests/testdata/foo.css');
assert.equal(error.loc, {
line: 1,
Expand Down Expand Up @@ -414,4 +414,29 @@ test('should support throwing in visitors', async () => {
assert.equal(error.message, 'Some error');
});

test('external import', async () => {
const { code: buffer } = await bundleAsync(/** @type {import('../index').BundleAsyncOptions} */ ({
filename: 'tests/testdata/has_external.css',
resolver: {
resolve(specifier, originatingFile) {
if (specifier === './does_not_exist.css' || specifier.startsWith('https:')) {
return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, I made resolve to accept string | true to focus on the rough design. I think string | { specifier: string, external?: boolean } would be nice here. What do you think?

}
return path.resolve(path.dirname(originatingFile), specifier);
}
}
}));
const code = buffer.toString('utf-8').trim();

const expected = `
@import "https://fonts.googleapis.com/css2?family=Roboto&display=swap";
@import "./does_not_exist.css";

.b {
height: calc(100vh - 64px);
}
`.trim();
if (code !== expected) throw new Error(`\`testResolver()\` failed. Expected:\n${expected}\n\nGot:\n${code}`);
});

test.run();
Loading