From 53e6619c4286bbb497fd527cb9068d9b5fc96af9 Mon Sep 17 00:00:00 2001 From: Sami Jaber Date: Thu, 22 Dec 2022 16:29:49 -0400 Subject: [PATCH] Fix: Vue `this` access in `provide()` (#964) * dedupe @babel/types * augment babel types * update yarn * comment * update yarn lock * add missing path for jsx-runtime * remove redundant type workaround * improve replace-identifiers * tweak * refactor, pass hook key * improve logging * debug * remove patch * fix fiddle * revert tsconfig changes * revert to yarn 3.2 * run yarn * add some missing deps for fiddle * undo @babel/types dedupe * remove * always add React.Fragment * bring back _this context workaround in vue * undo react change * use keys for Vue contexts * improve context check * fix for composition api too --- packages/cli/src/build/helpers/context.ts | 7 +- .../vue-composition.test.ts.snap | 28 ++++---- .../__tests__/__snapshots__/vue.test.ts.snap | 36 ++++++---- .../helpers/context-with-symbol-key.ts | 33 +++++++++ .../core/src/generators/context/svelte.ts | 33 +-------- packages/core/src/generators/context/vue.ts | 15 +--- .../core/src/generators/vue/compositionApi.ts | 21 +++--- packages/core/src/generators/vue/helpers.ts | 69 ++++++++++++------- .../core/src/generators/vue/optionsApi.ts | 21 +++++- 9 files changed, 153 insertions(+), 110 deletions(-) create mode 100644 packages/core/src/generators/context/helpers/context-with-symbol-key.ts diff --git a/packages/cli/src/build/helpers/context.ts b/packages/cli/src/build/helpers/context.ts index f3d3d9b491..7a04918303 100644 --- a/packages/cli/src/build/helpers/context.ts +++ b/packages/cli/src/build/helpers/context.ts @@ -33,7 +33,7 @@ export const generateContextFile = async ({ case 'vue': case 'vue2': case 'vue3': - return contextToVue(context); + return contextToVue(options.options[target])({ context }); case 'solid': return contextToSolid()({ context }); case 'react': @@ -46,7 +46,10 @@ export const generateContextFile = async ({ return contextToQwik()({ context }); default: console.warn('Context files are not supported for this target. Outputting no-op'); - return contextToVue(context); + return ` + // Noop file + export default {}; + `; } } }; diff --git a/packages/core/src/__tests__/__snapshots__/vue-composition.test.ts.snap b/packages/core/src/__tests__/__snapshots__/vue-composition.test.ts.snap index aa1440676e..a6ca2377eb 100644 --- a/packages/core/src/__tests__/__snapshots__/vue-composition.test.ts.snap +++ b/packages/core/src/__tests__/__snapshots__/vue-composition.test.ts.snap @@ -134,9 +134,9 @@ import { Injector, createInjector, MyService } from \\"@dummy/injection-js\\"; const name = ref(\\"PatrickJS\\"); -const myService = inject(MyService); +const myService = inject(MyService.key); -provide(Injector, createInjector()); +provide(Injector.key, createInjector()); const hi = myService.method(\\"hi\\"); console.log(hi); @@ -1746,15 +1746,15 @@ import Context2 from \\"@dummy/2\\"; const props = defineProps([\\"content\\"]); -const foo = inject(Context1); +const foo = inject(Context1.key); -provide(Context1, { +provide(Context1.key, { foo: \\"bar\\", content() { return props.content; }, }); -provide(Context2, { bar: \\"baz\\" }); +provide(Context2.key, { bar: \\"baz\\" }); " `; @@ -1770,7 +1770,7 @@ import BuilderContext from \\"@dummy/context.vue\\"; const props = defineProps([\\"content\\", \\"customComponents\\"]); -provide(BuilderContext, { +provide(BuilderContext.key, { content: props.content, registeredComponents: props.customComponents, }); @@ -2196,7 +2196,7 @@ import RenderBlocks from \\"@dummy/RenderBlocks.lite.tsx\\"; const props = defineProps([\\"content\\", \\"customComponents\\"]); -provide(BuilderContext, { +provide(BuilderContext.key, { get content() { return 3; }, @@ -2594,9 +2594,9 @@ import { Injector, createInjector, MyService } from \\"@dummy/injection-js\\"; const name = ref(\\"PatrickJS\\"); -const myService = inject(MyService); +const myService = inject(MyService.key); -provide(Injector, createInjector()); +provide(Injector.key, createInjector()); const hi = myService.method(\\"hi\\"); console.log(hi); @@ -4383,15 +4383,15 @@ import Context2 from \\"@dummy/2\\"; const props = defineProps(); -const foo = inject(Context1); +const foo = inject(Context1.key); -provide(Context1, { +provide(Context1.key, { foo: \\"bar\\", content() { return props.content; }, }); -provide(Context2, { bar: \\"baz\\" }); +provide(Context2.key, { bar: \\"baz\\" }); " `; @@ -4407,7 +4407,7 @@ import BuilderContext from \\"@dummy/context.vue\\"; const props = defineProps([\\"content\\", \\"customComponents\\"]); -provide(BuilderContext, { +provide(BuilderContext.key, { content: props.content, registeredComponents: props.customComponents, }); @@ -4914,7 +4914,7 @@ import RenderBlocks from \\"@dummy/RenderBlocks.lite.tsx\\"; const props = defineProps(); -provide(BuilderContext, { +provide(BuilderContext.key, { get content() { return 3; }, diff --git a/packages/core/src/__tests__/__snapshots__/vue.test.ts.snap b/packages/core/src/__tests__/__snapshots__/vue.test.ts.snap index 33e7f0667d..cad5da1aee 100644 --- a/packages/core/src/__tests__/__snapshots__/vue.test.ts.snap +++ b/packages/core/src/__tests__/__snapshots__/vue.test.ts.snap @@ -150,12 +150,13 @@ export default { data: () => ({ name: \\"PatrickJS\\" }), provide() { + const _this = this; return { - Injector: createInjector(), + [Injector.key]: createInjector(), }; }, inject: { - myService: MyService, + myService: MyService.key, }, created() { const hi = this.myService.method(\\"hi\\"); @@ -1936,10 +1937,11 @@ export default { props: [\\"content\\", \\"customComponents\\"], provide() { + const _this = this; return { - BuilderContext: { - content: this.content, - registeredComponents: this.customComponents, + [BuilderContext.key]: { + content: _this.content, + registeredComponents: _this.customComponents, }, }; }, @@ -2476,8 +2478,9 @@ export default { data: () => ({ trackClick }), provide() { + const _this = this; return { - BuilderContext: { + [BuilderContext.key]: { get content() { return 3; }, @@ -2934,12 +2937,13 @@ export default { data: () => ({ name: \\"PatrickJS\\" }), provide() { + const _this = this; return { - Injector: createInjector(), + [Injector.key]: createInjector(), }; }, inject: { - myService: MyService, + myService: MyService.key, }, created() { const hi = this.myService.method(\\"hi\\"); @@ -4964,10 +4968,11 @@ export default { props: [\\"content\\", \\"customComponents\\"], provide() { + const _this = this; return { - BuilderContext: { - content: this.content, - registeredComponents: this.customComponents, + [BuilderContext.key]: { + content: _this.content, + registeredComponents: _this.customComponents, }, }; }, @@ -5589,8 +5594,9 @@ export default { data: () => ({ trackClick }), provide() { + const _this = this; return { - BuilderContext: { + [BuilderContext.key]: { get content() { return 3; }, @@ -6023,8 +6029,9 @@ export default { data: () => ({ activeTab: 0 }), provide() { + const _this = this; return { - activeTab: this.activeTab, + [\\"activeTab\\"]: _this.activeTab, }; }, inject: { @@ -6402,8 +6409,9 @@ export default { data: () => ({ activeTab: 0 }), provide() { + const _this = this; return { - activeTab: this.activeTab, + [\\"activeTab\\"]: _this.activeTab, }; }, inject: { diff --git a/packages/core/src/generators/context/helpers/context-with-symbol-key.ts b/packages/core/src/generators/context/helpers/context-with-symbol-key.ts new file mode 100644 index 0000000000..d80abd27af --- /dev/null +++ b/packages/core/src/generators/context/helpers/context-with-symbol-key.ts @@ -0,0 +1,33 @@ +import { format } from 'prettier/standalone'; +import { stringifyContextValue } from '../../../helpers/get-state-object-string'; +import { MitosisContext } from '../../../types/mitosis-context'; +import { BaseTranspilerOptions } from '../../../types/transpiler'; + +export const getContextWithSymbolKey = + (options: Pick) => + ({ context }: { context: MitosisContext }): string => { + let str = ` + const key = Symbol(); + + export default { + ${context.name}: ${stringifyContextValue(context.value)}, + key + } + `; + + if (options.prettier !== false) { + try { + str = format(str, { + parser: 'typescript', + plugins: [ + require('prettier/parser-typescript'), // To support running in browsers + ], + }); + } catch (err) { + console.error('Format error for file:', str); + throw err; + } + } + + return str; + }; diff --git a/packages/core/src/generators/context/svelte.ts b/packages/core/src/generators/context/svelte.ts index 08bb6212e7..cc62473dfe 100644 --- a/packages/core/src/generators/context/svelte.ts +++ b/packages/core/src/generators/context/svelte.ts @@ -1,38 +1,9 @@ -import { format } from 'prettier/standalone'; -import { stringifyContextValue } from '../../helpers/get-state-object-string'; -import { MitosisContext } from '../../types/mitosis-context'; import { BaseTranspilerOptions } from '../../types/transpiler'; +import { getContextWithSymbolKey } from './helpers/context-with-symbol-key'; interface ContextToSvelteOptions extends Pick {} /** * TO-DO: support types */ -export const contextToSvelte = - (options: ContextToSvelteOptions = {}) => - ({ context }: { context: MitosisContext }): string => { - let str = ` - const key = Symbol(); - - export default { - ${context.name}: ${stringifyContextValue(context.value)}, - key - } - `; - - if (options.prettier !== false) { - try { - str = format(str, { - parser: 'typescript', - plugins: [ - require('prettier/parser-typescript'), // To support running in browsers - ], - }); - } catch (err) { - console.error('Format error for file:', str); - throw err; - } - } - - return str; - }; +export const contextToSvelte = getContextWithSymbolKey; diff --git a/packages/core/src/generators/context/vue.ts b/packages/core/src/generators/context/vue.ts index b8ce05ffad..46279d99aa 100644 --- a/packages/core/src/generators/context/vue.ts +++ b/packages/core/src/generators/context/vue.ts @@ -1,14 +1,3 @@ -import { MitosisContext } from '../../types/mitosis-context'; +import { getContextWithSymbolKey } from './helpers/context-with-symbol-key'; -type ContextToVueOptions = { - format?: boolean; -}; - -export function contextToVue(context: MitosisContext, options: ContextToVueOptions = {}): string { - let str = ` - // Noop file - export default {}; - `; - - return str; -} +export const contextToVue = getContextWithSymbolKey; diff --git a/packages/core/src/generators/vue/compositionApi.ts b/packages/core/src/generators/vue/compositionApi.ts index 3c195c3587..f16c0b4e8e 100644 --- a/packages/core/src/generators/vue/compositionApi.ts +++ b/packages/core/src/generators/vue/compositionApi.ts @@ -3,7 +3,7 @@ import json5 from 'json5'; import { pickBy } from 'lodash'; import { getStateObjectStringFromComponent } from '../../helpers/get-state-object-string'; import { MitosisComponent, extendedHook } from '../../types/mitosis-component'; -import { getContextValue } from './helpers'; +import { getContextKey, getContextValue } from './helpers'; import { ToVueOptions } from './types'; import { stripStateAndPropsRefs } from '../../helpers/strip-state-and-props-refs'; import { processBinding } from './helpers'; @@ -81,18 +81,19 @@ export function generateCompositionApiScript( ${props.length ? getCompositionPropDefinition({ component, props, options }) : ''} ${refs} - ${Object.keys(component.context.get) - ?.map((key) => `const ${key} = inject(${component.context.get[key].name})`) + ${Object.entries(component.context.get) + ?.map(([key, context]) => { + return `const ${key} = inject(${getContextKey(context)})`; + }) .join('\n')} ${Object.values(component.context.set) - ?.map( - (contextSet) => - `provide(${contextSet.name}, ${getContextValue({ - json: component, - options, - })(contextSet)})`, - ) + ?.map((contextSet) => { + const contextValue = getContextValue({ json: component, options })(contextSet); + const key = getContextKey(contextSet); + + return `provide(${key}, ${contextValue})`; + }) .join('\n')} ${Object.keys(component.refs) diff --git a/packages/core/src/generators/vue/helpers.ts b/packages/core/src/generators/vue/helpers.ts index ec8dc8f36b..451981868d 100644 --- a/packages/core/src/generators/vue/helpers.ts +++ b/packages/core/src/generators/vue/helpers.ts @@ -1,7 +1,7 @@ import { Nullable } from '../../helpers/nullable'; import { stringifyContextValue } from '../../helpers/get-state-object-string'; import { stripStateAndPropsRefs } from '../../helpers/strip-state-and-props-refs'; -import { ContextSetInfo, MitosisComponent } from '../../types/mitosis-component'; +import { ContextGetInfo, ContextSetInfo, MitosisComponent } from '../../types/mitosis-component'; import { MitosisNode } from '../../types/mitosis-node'; import { ToVueOptions } from './types'; import { pipe } from 'fp-ts/lib/function'; @@ -106,14 +106,24 @@ const getAllRefs = (component: MitosisComponent) => { return allKeys; }; -function processRefs(input: string, component: MitosisComponent, options: ToVueOptions) { +function processRefs({ + input, + component, + options, + thisPrefix, +}: { + input: string; + component: MitosisComponent; + options: ToVueOptions; + thisPrefix: ProcessBinding['thisPrefix']; +}) { const refs = options.api === 'options' ? getContextNames(component) : getAllRefs(component); return babelTransformExpression(input, { Identifier(path: babel.NodePath) { const name = path.node.name; if (refs.includes(name) && shouldAppendValueToRef(path)) { - const newValue = options.api === 'options' ? `this.${name}` : `${name}.value`; + const newValue = options.api === 'options' ? `${thisPrefix}.${name}` : `${name}.value`; path.replaceWith(types.identifier(newValue)); } }, @@ -134,6 +144,14 @@ function prefixMethodsWithThis(input: string, component: MitosisComponent, optio } } +type ProcessBinding = { + code: string; + options: ToVueOptions; + json: MitosisComponent; + preserveGetter?: boolean; + thisPrefix?: 'this' | '_this'; +}; + // TODO: migrate all stripStateAndPropsRefs to use this here // to properly replace context refs export const processBinding = ({ @@ -141,12 +159,8 @@ export const processBinding = ({ options, json, preserveGetter = false, -}: { - code: string; - options: ToVueOptions; - json: MitosisComponent; - preserveGetter?: boolean; -}): string => { + thisPrefix = 'this', +}: ProcessBinding): string => { try { return pipe( stripStateAndPropsRefs(code, { @@ -160,15 +174,20 @@ export const processBinding = ({ return name; case 'options': if (name === 'children' || name.startsWith('children.')) { - return 'this.$slots.default'; + return '${thisPrefix}.$slots.default'; } - return `this.${name}`; + return `${thisPrefix}.${name}`; } }, }), - (code) => processRefs(code, json, options), - (code) => prefixMethodsWithThis(code, json, options), - (code) => (preserveGetter === false ? stripGetter(code) : code), + (x) => { + return pipe( + x, + (code) => processRefs({ input: code, component: json, options, thisPrefix }), + (code) => prefixMethodsWithThis(code, json, options), + (code) => (preserveGetter === false ? stripGetter(code) : code), + ); + }, ); } catch (e) { console.log('could not process bindings in ', { code }); @@ -177,23 +196,27 @@ export const processBinding = ({ }; export const getContextValue = - ({ options, json }: { options: ToVueOptions; json: MitosisComponent }) => + (args: Pick) => ({ name, ref, value }: ContextSetInfo): Nullable => { const valueStr = value ? stringifyContextValue(value, { - valueMapper: (code) => processBinding({ code, options, json, preserveGetter: true }), + valueMapper: (code) => processBinding({ code, ...args, preserveGetter: true }), }) : ref - ? processBinding({ code: ref, options, json, preserveGetter: true }) + ? processBinding({ code: ref, ...args, preserveGetter: true }) : null; return valueStr; }; -export const getContextProvideString = (json: MitosisComponent, options: ToVueOptions) => { - return `{ - ${Object.values(json.context.set) - .map((setVal) => `${setVal.name}: ${getContextValue({ options, json })(setVal)}`) - .join(',')} - }`; +export const checkIfContextHasStrName = (context: ContextGetInfo | ContextSetInfo) => { + // check if the name is wrapped in single or double quotes + const isStrName = context.name.startsWith("'") || context.name.startsWith('"'); + return isStrName; +}; + +export const getContextKey = (context: ContextGetInfo | ContextSetInfo) => { + const isStrName = checkIfContextHasStrName(context); + const key = isStrName ? context.name : `${context.name}.key`; + return key; }; diff --git a/packages/core/src/generators/vue/optionsApi.ts b/packages/core/src/generators/vue/optionsApi.ts index 0f6ab20899..a87f437513 100644 --- a/packages/core/src/generators/vue/optionsApi.ts +++ b/packages/core/src/generators/vue/optionsApi.ts @@ -7,15 +7,29 @@ import { checkIsDefined } from '../../helpers/nullable'; import { checkIsComponentImport } from '../../helpers/render-imports'; import { MitosisComponent, extendedHook } from '../../types/mitosis-component'; import { PropsDefinition, DefaultProps } from 'vue/types/options'; -import { encodeQuotes, getContextProvideString, getOnUpdateHookName } from './helpers'; +import { encodeQuotes, getContextKey, getContextValue, getOnUpdateHookName } from './helpers'; import { ToVueOptions } from './types'; +const getContextProvideString = (json: MitosisComponent, options: ToVueOptions) => { + return `{ + ${Object.values(json.context.set) + .map((setVal) => { + const key = getContextKey(setVal); + return `[${key}]: ${getContextValue({ options, json, thisPrefix: '_this' })(setVal)}`; + }) + .join(',')} + }`; +}; + function getContextInjectString(component: MitosisComponent, options: ToVueOptions) { let str = '{'; - for (const key in component.context.get) { + const contextGetters = component.context.get; + + for (const key in contextGetters) { + const context = contextGetters[key]; str += ` - ${key}: ${encodeQuotes(component.context.get[key].name)}, + ${key}: ${encodeQuotes(getContextKey(context))}, `; } @@ -187,6 +201,7 @@ export function generateOptionsApiScript( ${ size(component.context.set) ? `provide() { + const _this = this; return ${getContextProvideString(component, options)} },` : ''