-
Notifications
You must be signed in to change notification settings - Fork 64
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
Reuse same object for tagged template quasis. #67
Conversation
Spec tagged template literals pass the same object to each invocation of the template tag function. This way that object can be used as a cache key, for example, using a `Map`, so that parsing of the string contents can be cached. This patch changes the tagged template literal transform to declare a frozen top-level array with the quasi strings, and passes that array to each invocation. I think it's important to adhere to the spec re: freezing, because otherwise people might try to attach properties to the object itself as a sort of caching mechanism, which doesn't work in spec tagged template literals. I'm unsure if this is the _best_ way to declare a top-level variable, but it seems to work OK :)
At least my Node v6.12.0 and my Firefox 57 seem to intern the arrays, so that even different call sites pass the same object: let x;
function f(r) { x = x || r; console.log(x === r) };
f`a${ 1 }`; // true
f`a${ x }`; // true
f`b`; // false
f(["a"]); // false
f`a${ 5 }` // true |
oh yeah that's a great catch, thanks! 6272cc0 only creates a new |
Wow, that was fast! This doesn't work across multiple files, but otherwise it's great, and I doubt there is a reasonable way to make that work. |
yeah, exactly—and even so, this will still allow template tags to save a lot of time by reducing parsing to just once per template per file, instead of on every single call. |
Ok, I just went ahead and merged this, it's definitely an improvement :) Thanks for your contribution! |
I just pushed 7583ae8, which fixed this code putting definitions before directives. |
oops! thanks! |
bublejs/buble#67 changed Bublé template strings to compile similarly to Babel ones, for spec reasons. With this patch, the new style as well as the old style are detected correctly.
Actually, Runtime Semantics: GetTemplateObject step 4.a specifies that each parse node gets its own template object, contrary to what V8 does. See also tc39/test262#972. |
O, that's nice. I can prepare a PR that basically reverts the second commit from this one today, I think. |
Spec tagged template literals pass the same object to each invocation of
the template tag function. This way that object can be used as a cache
key, for example, using a
Map
, so that parsing of the string contentscan be cached.
This patch changes the tagged template literal transform to declare a
frozen top-level array with the quasi strings, and passes that array to
each invocation. If this lands I think it's important to adhere to the spec re:
freezing, because otherwise people might try to attach properties to the
object itself as a sort of caching mechanism, which doesn't work in spec
tagged template literals.
I'm unsure if this is the best way to declare a top-level variable,
but it seems to work OK :)