yansenlei / VJsoneditor

:large_blue_diamond: vue use jsoneditor
https://yansenlei.github.io/VJsoneditor/
Apache License 2.0
182 stars 45 forks source link

feat: allow options.mode to be reactive #25

Closed ferm10n closed 3 years ago

ferm10n commented 4 years ago

Hi @yansenlei ,

I needed the ability to switch between edit and view mode, but the editor mode was not set up to be reactive. I made a small edit that watches the original options prop passed in, and updates the editor mode when options.mode changes. Please let me know if this is acceptable, or if other changes should be made for it. Thanks!

ferm10n commented 4 years ago

Hey @yansenlei I noticed you made a recent release for this package. Any chance you could take a look at this merge request?

ogiermaitre commented 4 years ago

I would also be interested by this merge.

yansenlei commented 3 years ago

ok, I'll see to it right away

yansenlei commented 3 years ago

Hi, @ferm10n I have tried, I think it's more flexible to $refs.instance.

https://codesandbox.io/s/vjsoneditor-change-mode-r94w0?file=/src/App.vue:1121-1165

<v-jsoneditor ref="JsonEditor" v-model="json"/>

this.$refs.JsonEditor.editor.setMode('code')
yansenlei commented 3 years ago

😃 v-jsoneditor can also provide convenience, But need to consider scalability.

watch: {
    options: {
      handler (newValue, oldValue) {
        const diff = getDiffKeys(newValue, oldValue)
        if (this.editor && diff.includes('mode')) {
          this.editor.setMode(this.options.mode)
        }
      },
    }
}
ferm10n commented 3 years ago

Using $refs to access VJsoneditor's editor instance is a bad idea. As a library maintainer, you usually want to have control over how other devs interact with your lib. Some people would call these "module boundaries". While it's possible to manipulate editor with $refs, it makes it infinitely harder for you as the maintainer to determine what is and what is not a breaking change in the future. This is why Vue has props and events, it forces components to have a well defined data direction flow. Modifying component internals with $refs is too hacky IMO.

One way to fix this though might be declaring exactly what the vjsoneditor module provides, like adding an index.d.ts like this:

// src/index.d.ts
import _Vue, { ComponentOptions, PluginObject } from 'vue';
import JSONEditor from 'jsoneditor';

type VJsoneditorType<V extends _Vue = _Vue> = PluginObject<V> & ComponentOptions<V, {
    editor: JSONEditor | null;
}>;

export const VJsoneditor: VJsoneditorType;
export default VJsoneditor;

and adding the types reference in package.json

{
  "types": "./src/index.d.ts"
}

That way, you can still have the module boundaries well defined. Note, I'm not 100% sure this is the correct way to write the typescript for this. What do you think?

ferm10n commented 3 years ago

It looks like you cherry picked this commit last October! I didn't know that happened, I've had my team using the fork I made this whole time 😅

So I think this pull request can be closed