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.

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

Make it TypeScript compatible #19

Closed
apirogov opened this issue Sep 2, 2021 · 17 comments
Closed

Make it TypeScript compatible #19

apirogov opened this issue Sep 2, 2021 · 17 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@apirogov
Copy link

apirogov commented Sep 2, 2021

I am using typescript in my project and e.g. when I use "createAjvValidator" and pass it a schema, TypeScript complains because due to the comment in the code it thinks there is some JSON type and my data is not of that type.

It would be great if the exposed functions would be made properly TypeScript compatible, or at least fixed in a way that TypeScript is not confused and treats everything as any instead of non-existing types.

@josdejong josdejong added enhancement New feature or request help wanted Extra attention is needed labels Sep 9, 2021
@josdejong
Copy link
Owner

Yes, I would like to convert the source code to TypeScript.

This requires some fiddling with the build setup to get this all working though. Help would be welcome.

@cloakedninjas
Copy link

So - now that v6 is released and in lieu of source being in TypeScript - will there be .d.ts file provided? DefinitelyTyped is missing this project

@josdejong
Copy link
Owner

Yes the plan is to make this a TypeScript project. The Svelte setup is TypeScript-ready but there where some issues getting the mocha tests working with TypeScript files.

Help would be welcome.

@josdejong
Copy link
Owner

I've fixed the setup so it all works with TypeScript. I've converted a couple of files already, though most files still have to be converted. We'll get there 😄

@cloakedninjas
Copy link

cloakedninjas commented May 24, 2022

Thanks for the update, but ideally the types.d.ts file gets placed alongside the dist file

Doing

import { JSONEditor } from 'svelte-jsoneditor/dist/jsoneditor';

Still results in TS7016: Could not find a declaration file for module 'svelte-jsoneditor/dist/jsoneditor'

And once that's created, a module or interface will be needed for the whole library too, e.g.

export declare interface SvelteComponentConstructor {
    target: HTMLElement;
    props?: JsonEditorOptions;
}

Cheers 👍

@cloakedninjas
Copy link

Also - The types.d.ts file that's included in the npm package is not the same as the src/lib/types.ts - The @ts-ignore annotation is missing and now after installing the latest (0.3.53) version, fails to compile:

TS2304: Cannot find name 'SvelteComponentConstructor'.

@josdejong
Copy link
Owner

Yes indeed, there is work to be done. A couple of things:

  • Step one was to make the setup (build tools and testing) work with TS, that is now down,
  • So now we can gradually convert all code to TS (working on that)
  • The Svelte output has proper TS definitions and should work as is
  • The vanilla JS bundle output dist/jsoneditor.js is missing TS definitions.
    • I want to split the npm package in two (one for Svelte, one for Vanilla JS), in order to prevent conflicts between the two and issues with build tools because of that
    • I need to get proper type definitions out of the rollup bundler that generates dist/jsoneditor.js. Fiddling around with that.

@josdejong
Copy link
Owner

Things should be improved a lot in v0.3.54. Not sure if it works for all setups but the TS definitions work in a standard SvelteKit application and a standard TypeScript create-react-app.

Can you give it a try @cloakedninjas?

@josdejong
Copy link
Owner

I see when using the bundle, the type definitions can be found, but in a typical setup you get a lot of warnings about missing source maps (pointing to TypeScript source files that are not part of the npm package). Looking into it.

@cloakedninjas
Copy link

I see when using the bundle, the type definitions can be found, but in a typical setup you get a lot of warnings about missing source maps (pointing to TypeScript source files that are not part of the npm package). Looking into it.

Just installed 0.3.58 and first thing that I noticed was:

export default class JsonEditor extends SvelteComponentTyped<JsonEditorProps, JsonEditorEvents, JsonEditorSlots> {

Which relies on

import { SvelteComponentTyped } from "svelte";

in node_modules/svelte-jsoneditor/components/JSONEditor.svelte.d.ts

But we don't use Svelte in our project

@josdejong
Copy link
Owner

Thanks for trying it out. That is a good point. Those interfaces are indeed defined in the Svelte package. So either you will have to import svelte just for the type definitions, or we somehow should copy those interfaces so you don't have to import svelte.

Help would be welcome.

@cloakedninjas
Copy link

If auto-generating these definitions is proving tricky, then I would suggest crafting a manual .d.ts file that matches your API and either bundling it the npm package or submitting it to DefinitelyTyped under @types/svelte-jsoneditor

Since I don't image Svelte should be a required dependency for users of the ES bundle

@josdejong
Copy link
Owner

Yes indeed, the library should not require installing svelte.

Manually crafting the definitions could be an option indeed, though auto generating them has my preference as a maintainer of the library 😁 . The definitions are part of the library itself and I think we should not move them to a separate @types/svelte-jsoneditor library.

@josdejong
Copy link
Owner

To solve the missing TypeScript definitions of svelte, I've made svelte a dependency of the project now in v0.4.0. It feels not ideal and it is only needed for the types, but I think having an ugly working solution is better than having no solution. In a next step, I would like to see if we can output type definitions alongside the bundle to make the svelte dependency redundant again.

Also, I managed to fix the sourcemap, so that is included again in v0.4.0 too.

Can you guys give it a try?

@josdejong
Copy link
Owner

I've just published v0.5.0, which is split in two npm packages:

  • svelte-jsoneditor for usage in Svelte
  • vanilla-jsoneditor for use in plain JavaScript and frameworks like SolidJS, React, Vue, etc

The vanilla-jsoneditor package has zero dependencies. The types of dependencies like svelte and immutable-json-patch are embedded too to achieve that.

I'm closing this issue now. If there are still TS related issues please reopen a new issue.

@CarterFendley
Copy link

Pardon for posting after closed, but are there typings for the vanilla-jsoneditor? I am using this library with VueJS & TS and ran into issues when attempting to upgrade.

@josdejong
Copy link
Owner

@CarterFendley yes there are typings for vanilla-jsoneditor, it should work. If you look in https://unpkg.com/browse/vanilla-jsoneditor@0.6.3/ you will see the file index.d.ts listed, and this is referenced in package.json in the field "types": "index.d.ts".

I did a little test in a new react typescript app to see if I get types, that works as far as I can see:

afbeelding

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants