-
Notifications
You must be signed in to change notification settings - Fork 28
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
fix(vue): transpile vue template #289
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #289 +/- ##
==========================================
+ Coverage 82.86% 84.38% +1.51%
==========================================
Files 12 13 +1
Lines 852 1082 +230
Branches 133 232 +99
==========================================
+ Hits 706 913 +207
- Misses 144 167 +23
Partials 2 2 ☔ View full report in Codecov by Sentry. |
i wonder if this will also fix a bug i haven't yet had time to report here (can be reproduced in nuxt/image repo) it was compiling nuxtimg component to reference properties in the template that were not exposed from setup |
I built it with The pr just remove ts syntax from vue template so it should has no effect to this.
Vue expose all props and variables defined in |
you can replicate by running |
I replicated it, minimal reproduction vue playground <script>
import { defineComponent } from "vue";
export default defineComponent({
setup() {
const isServer = true;
const __returned__ = { isServer };
Object.defineProperty(__returned__, "__isScriptSetup", { enumerable: false, value: true });
return __returned__;
}
});
</script>
<template>
{{ isServer || 'nothing here' }}
</template> once I removed Then I found the description about __isScriptSetup: compileScript.ts
This is weird, according to the comments it should expose more stuff. But later I found this: const hasSetupBinding = (state: Data, key: string) =>
state !== EMPTY_OBJ && !state.__isScriptSetup && hasOwn(state, key) It looks like we also need to compile the template and get these variables in |
Now it seems a bit of a dilemma:
|
now for files with no As for point 2 in #289 (comment), I think make our own transpiler requires thousands lines of code to handling edge cases, it would be better to make it in upstream if we really want. Tested with nuxt/image, now it passed all test. build log
|
Thanks so much for your great work! I've also stumbled upon this issue today and was really delighted to see it already being taken care of. This seems to be breaking when using a prop with no value, which will normally imply The
|
You are right, the project didn't enable |
We also ran into the problem Daniel described, for now this blocks us from updating mkdist past 1.6.0. |
Ideally it’d be great to disable compilation entirely and simply copy the file as is. Not sure if this is already possible, but we definitely have a use case for it. |
@robinscholz This pr also fixed Daniel's problem, see #289 (comment) As for disabling compilation, you can set only the loader you need to transform in options, for example: mkdist({ loaders: ['js', 'postcss', 'sass'] }) it will copy |
resolves #281
Transpile vue template if the file have a typescript script block