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

Jump to bottom

Strip hygiene code in favor of UUIDs #83

Merged
merged 2 commits into from
Nov 5, 2024
Merged

Strip hygiene code in favor of UUIDs #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f306a1b
Strip hygiene code in favor of UUIDs
wagenet Oct 31, 2024
8d8cb16
Fix node tests
wagenet Oct 31, 2024
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
28 changes: 17 additions & 11 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,16 @@ serde-wasm-bindgen = "0.4"
wasm-bindgen = "0.2.95"
js-sys = "0.3.64"

[dependencies.uuid]
version = "1.11.0"
features = [
"v4", # Lets you generate random UUIDs
"fast-rng" # Use a faster (but still sufficiently random) RNG
]

[dev-dependencies]
difference = "2"
regex = "1.11.1"



127 changes: 33 additions & 94 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ use swc_ecma_ast::{
};
use swc_ecma_codegen::Emitter;
use swc_ecma_parser::{lexer::Lexer, Parser, StringInput, Syntax, TsConfig};
use swc_ecma_transforms::hygiene::hygiene_with_config;
use swc_ecma_transforms::resolver;
use swc_ecma_utils::private_ident;
use swc_ecma_visit::{as_folder, VisitMutWith, VisitWith};
use uuid::Uuid;

mod bindings;
mod snippets;
Expand Down Expand Up @@ -116,16 +116,14 @@ impl Preprocessor {
GLOBALS.set(&Default::default(), || {
let mut parsed_module = parser.parse_module()?;

let found_id = find_existing_import(&parsed_module, target_module, target_specifier);
let had_id_already = found_id.is_some();
let id = found_id.unwrap_or_else(|| private_ident!(target_specifier));
let id = private_ident!(format!("{}_{}", target_specifier, Uuid::new_v4().to_string().replace("-", "")));
let mut needs_import = false;
parsed_module.visit_mut_with(&mut as_folder(transform::TransformVisitor::new(
&id,
Some(&mut needs_import),
)));

if !had_id_already && needs_import {
if needs_import {
insert_import(&mut parsed_module, target_module, target_specifier, &id)
}

Expand All @@ -134,16 +132,6 @@ impl Preprocessor {

parsed_module.visit_mut_with(&mut resolver(unresolved_mark, top_level_mark, false));

let mut h = hygiene_with_config(swc_ecma_transforms::hygiene::Config {
keep_class_names: true,
top_level_mark,
safari_10: false,
ignore_eval: false,
});
parsed_module.visit_mut_with(&mut h);

simplify_imports(&mut parsed_module);

Ok(self.print(&parsed_module, options.inline_source_map))
})
}
Expand Down Expand Up @@ -192,38 +180,6 @@ impl Preprocessor {
}
}

fn find_existing_import(
parsed_module: &Module,
target_module: &str,
target_specifier: &str,
) -> Option<Ident> {
for item in parsed_module.body.iter() {
match item {
ModuleItem::ModuleDecl(ModuleDecl::Import(import_declaration)) => {
if import_declaration.src.value.to_string() == target_module {
for specifier in import_declaration.specifiers.iter() {
match specifier {
ImportSpecifier::Named(s) => {
let imported = match &s.imported {
Some(ModuleExportName::Ident(i)) => i.sym.to_string(),
Some(ModuleExportName::Str(s)) => s.value.to_string(),
None => s.local.sym.to_string(),
};
if imported == target_specifier {
return Some(s.local.clone());
}
}
_ => {}
}
}
}
}
_ => {}
}
}
None
}

fn insert_import(
parsed_module: &mut Module,
target_module: &str,
Expand All @@ -250,39 +206,6 @@ fn insert_import(
);
}

// It's not until after the hygiene pass that we know what local name is being
// used for our import. If it turns out to equal the imported name, we can
// implify from "import { template as template } from..." down to "import {
// template } from ...".
fn simplify_imports(parsed_module: &mut Module) {
for item in parsed_module.body.iter_mut() {
match item {
ModuleItem::ModuleDecl(ModuleDecl::Import(import_declaration)) => {
for specifier in import_declaration.specifiers.iter_mut() {
match specifier {
ImportSpecifier::Named(specifier) => {
if let ImportNamedSpecifier {
imported: Some(ModuleExportName::Ident(imported)),
local,
..
} = specifier
{
if local.sym == imported.sym {
specifier.imported = None;
}
}
}
_ => {}
}
}
}
_ => {}
}
}
}



#[cfg(test)]
mod test_helpers;

Expand All @@ -299,24 +222,26 @@ macro_rules! testcase {
testcase! {
no_preexisting_import,
r#"let x = <template>hello</template>"#,
r#"import { template } from "@ember/template-compiler";
let x = template(`hello`, { eval() { return eval(arguments[0])} });"#
r#"import { template as template_UUID } from "@ember/template-compiler";
let x = template_UUID(`hello`, { eval() { return eval(arguments[0])} });"#
}

testcase! {
uses_preexisting_import,
preexisting_import,
r#"import { template } from "@ember/template-compiler";
let x = <template>hello</template>"#,
r#"import { template } from "@ember/template-compiler";
let x = template(`hello`, { eval() { return eval(arguments[0])} });"#
r#"import { template as template_UUID } from "@ember/template-compiler";
import { template } from "@ember/template-compiler";
let x = template_UUID(`hello`, { eval() { return eval(arguments[0])} });"#
}

testcase! {
uses_preexisting_renamed_import,
preexisting_renamed_import,
r#"import { template as t } from "@ember/template-compiler";
let x = <template>hello</template>"#,
r#"import { template as t } from "@ember/template-compiler";
let x = t(`hello`, { eval() { return eval(arguments[0])} })"#
r#"import { template as template_UUID } from "@ember/template-compiler";
import { template as t } from "@ember/template-compiler";
let x = template_UUID(`hello`, { eval() { return eval(arguments[0])} })"#
}

testcase! {
Expand All @@ -330,10 +255,10 @@ testcase! {
r#"function template() {};
console.log(template());
export default <template>Hi</template>"#,
r#"import { template as template1 } from "@ember/template-compiler";
r#"import { template as template_UUID } from "@ember/template-compiler";
function template() {};
console.log(template());
export default template1(`Hi`, { eval() { return eval(arguments[0])} });"#
export default template_UUID(`Hi`, { eval() { return eval(arguments[0])} });"#
}

testcase! {
Expand All @@ -342,10 +267,10 @@ testcase! {
console.log(template);
return <template>X</template>;
};"#,
r#"import { template as template1 } from "@ember/template-compiler";
r#"import { template as template_UUID } from "@ember/template-compiler";
export default function(template) {
console.log(template);
return template1(`X`, { eval() { return eval(arguments[0])} });
return template_UUID(`X`, { eval() { return eval(arguments[0])} });
};"#
}

Expand All @@ -355,9 +280,23 @@ testcase! {
console.log(message);
return <template>hello</template>
}"#,
r#"import { template } from "@ember/template-compiler";
r#"import { template as template_UUID } from "@ember/template-compiler";
function makeComponent(message: string) {
console.log(message);
return template(`hello`, { eval() { return eval(arguments[0]) } });
return template_UUID(`hello`, { eval() { return eval(arguments[0]) } });
}"#
}

testcase! {
handles_typescript_this,
r#"function f(this: Context, ...args: unknown[]) {
function t(this: Context, ...args: unknown[]) {};
return <template></template>
}"#,
r#"import { template as template_UUID } from "@ember/template-compiler";
function f(this: Context, ...args: unknown[]) {
function t(this: Context, ...args: unknown[]) {}
;
return template_UUID(``, { eval() { return eval(arguments[0]) } });
}"#
}
7 changes: 5 additions & 2 deletions src/test_helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use difference::Changeset;
use regex::Regex;
use swc_common::comments::SingleThreadedComments;
use swc_common::{self, sync::Lrc, FileName, SourceMap};
use swc_ecma_codegen::Emitter;
Expand All @@ -7,13 +8,15 @@ use swc_ecma_parser::{lexer::Lexer, Parser, StringInput, Syntax, TsConfig};
use crate::Preprocessor;

pub fn testcase(input: &str, expected: &str) -> Result<(), swc_ecma_parser::error::Error> {
let re = Regex::new(r"template_[0-9a-f]{32}").unwrap();
let p = Preprocessor::new();
let actual = p.process(input, Default::default())?;
let actual_santized = re.replace_all(&actual, "template_UUID");
let normalized_expected = normalize(expected);
if actual != normalized_expected {
if actual_santized != normalized_expected {
panic!(
"code differs from expected:\n{}",
format!("{}", Changeset::new(&actual, &normalized_expected, "\n"))
format!("{}", Changeset::new(&actual_santized, &normalized_expected, "\n"))
);
}
Ok(())
Expand Down
20 changes: 12 additions & 8 deletions test/process.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,17 @@ const { expect } = chai;

const p = new Preprocessor();

function normalizeOutput(output) {
return output.replace(/template_[0-9a-f]{32}/g, "template_UUID");
}

describe(`process`, function () {
it("works for a basic example", function () {
let output = p.process("<template>Hi</template>");

expect(output).to
.equalCode(`import { template } from "@ember/template-compiler";
export default template(\`Hi\`, {
expect(normalizeOutput(output)).to
.equalCode(`import { template as template_UUID } from "@ember/template-compiler";
export default template_UUID(\`Hi\`, {
eval () {
return eval(arguments[0]);
}
Expand All @@ -32,12 +36,12 @@ describe(`process`, function () {

let output = p.process(input);

expect(output).to.equalCode(
`import { template } from "@ember/template-compiler";
let Foo = class Foo extends Component {
expect(normalizeOutput(output)).to.equalCode(
`import { template as template_UUID } from "@ember/template-compiler";
class Foo extends Component {
greeting = 'Hello';
static{
template(\`{{this.greeting}}, \\\`lifeform\\\`!\`, {
template_UUID(\`{{this.greeting}}, \\\`lifeform\\\`!\`, {
component: this,
eval () {
return eval(arguments[0]);
Expand Down Expand Up @@ -94,7 +98,7 @@ describe(`process`, function () {
let output = p.process(`<template>Hi</template>`, { inline_source_map: true });

expect(output).to.match(
/sourceMappingURL=data:application\/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIjxhbm9uPiJdLCJzb3VyY2VzQ29udGVudCI6WyI8dGVtcGxhdGU-SGk8L3RlbXBsYXRlPiJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiO0FBQUEsZUFBQSxTQUFVLENBQUEsRUFBRSxDQUFBLEVBQUE7SUFBQTtRQUFBLE9BQUEsS0FBQSxTQUFBLENBQUEsRUFBVztJQUFEO0FBQUEsR0FBQyJ9/
/sourceMappingURL=data:application\/json;base64,/
Copy link
Contributor

Choose a reason for hiding this comment

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

is this right? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I know nothing about source maps.

and... currently our gjs/gts sourcemaps are very weird in-browser

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's pretty hard to check the exact source map now that the template imports gets a UUID. This at least confirms that we did get an inline source map of some sort.

Copy link
Contributor

Choose a reason for hiding this comment

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

We may need to use magic-string or something (whatever the SWC equiv is) to keep a correct sourcemap. I've just figured out how to get gjs/gts sourcemaps in the browser over here: embroider-build/embroider#2162

Copy link

Choose a reason for hiding this comment

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

This looks like an empty source map though

Copy link
Contributor

Choose a reason for hiding this comment

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

what I missed is that this is just a regex matcher -- it's less precise, but ensures that the sourcemap is present, but doesn't check content

Copy link

Choose a reason for hiding this comment

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

Ah, got it

);
});
});
Loading
Loading