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

Why add both tsconfig.json and jsconfig.json? #73

Open
ericdrobinson opened this issue Dec 31, 2019 · 15 comments
Open

Why add both tsconfig.json and jsconfig.json? #73

ericdrobinson opened this issue Dec 31, 2019 · 15 comments
Assignees

Comments

@ericdrobinson
Copy link

ericdrobinson commented Dec 31, 2019

Declaring both tsconfig.json and jsconfig.json is unnecessary (and actually a bit confusing). The presence of either will instruct the TypeScript Language Service that the containing folder is the root of a "Project" (for which the config file is the ruleset by which to process the project). If you specify both, then things become ambiguous...

I would recommend removing the tsconfig.json file. This is due to the following reasons:

  1. The included sample file is JavaScript.
  2. Most users [at time of writing] will probably start by using JavaScript projects.
  3. If a user is using TypeScript, they probably already know about tsconfig.json and may have it set up for themselves.
  4. The current tsconfig.json file makes certain assumptions that will not hold true for all projects:
    1. The user's TypeScript project requires support for JavaScript (allowJS and checkJS settings).
    2. The user's TypeScript project has node_modules installed (see: types and typeRoots properties).
    3. The user's TypeScript project actually needs to specify the types and typeRoots properties at all.

To test a "minimal TypeScript project", I created a project that contained a src folder with a single test.ts file, the types folder with all declarations in this project, and a tsconfig.json file with the following contents:

{
  "compilerOptions": {
    "target": "es2015",
    "module": "CommonJS",
    "lib": [
        "es2015",
        "dom"
    ]
  }
}

All type declarations worked as expected and the compilation process proceeded without issue.

Rather than including a tsconfig.json file, it may be worthwhile to include a "Using these types with TypeScript" section in the ReadMe that calls out the settings above and explains why they're there. Specifically:

  • Specify "target": "es2015" because XD supports ES2015.
  • Specify "module": "CommonJS" because XD uses require for importing libraries (at least it does so in the documentation).
  • Specify "lib": ["es2015", "dom"] because XD supports JavaScript types specified in ES2015 and much of the types declared in the standard DOM library.

Such a section might go on to suggest that:

Adding the types folder from this project to the same folder that contains your tsconfig.json file will allow TypeScript scripts to resolve the XD types. However, if your tsconfig.json file contains a types or typeRoots property, you may need to add the types folder to one or both of those settings to allow the TypeScript language service and compiler to properly resolve the types.

Does this make sense?


SIDE NOTE: I added the module property to the JSON (as compared with the current tsconfig.json file, as TypeScript will output ES6-style module import statements rather than require, which I'm not sure if XD/UXP supports yet or not (doesn't appear to be listed in the documentation in any way). Specifying CommonJS for the module property instructs the TypeScript compiler to output JavaScript code that uses require.

If ES6-style modules are actually supported, then removing this property would be spiffy.

@pklaschka
Copy link
Contributor

@ericdrobinson
You know, the funny thing is that I thought about doing it the other way around. I've noticed that a slightly adjusted tsconfig.json allows a plug-and-play functionality in WebStorm as well (I'm in the process of open-sourcing my Lorem Ipsum plugin, which is why I'm refactoring a lot and today stumbled upon this).

In said plugins, the following tsconfig.json allowed full autocompletion in a plugin that follows my boilerplate structure (I'll look into adjusting it for a more general use-case):

{
  "compilerOptions": {
    "module": "commonjs",
    "strict": true,
    "target": "es2016",
    "baseUrl": "./",
    "allowJs": true,
    "checkJs": true,
    "resolveJsonModule": true,
    "lib": [
      "es2015",
      "dom"
    ]
  },
  "include": [
    "src/**/*",
    "types/**/*"
  ]
}

This appears to give me full plug-and-play autocompletion goodness in both WebStorm and VSCode (and probably a lot of other editors, jsconfig.json seems to be a VSCode-specific thing, after all).

Since this would basically simplify using it in projects for usage with Typescript and many (more?) editors, at first glance, it sounds like the preferable solution to me.

For usage with typescript, I'd suggest having it used as a dependency

{
  "@types/adobe-xd": "git://github.com/AdobeXD/typings.git"
}

is the preferred solution (although, for some reason I haven't found in two hours of debugging, this doesn't grant me access to global interfaces like XDSelection; that only gets resolved when I copy the types folder into the project...)

Any thoughts?


SIDE NOTE: I added the module property to the JSON (as compared with the current tsconfig.json file, as TypeScript will output ES6-style module import statements rather than require, which I'm not sure if XD/UXP supports yet or not (doesn't appear to be listed in the documentation in any way). Specifying CommonJS for the module property instructs the TypeScript compiler to output JavaScript code that uses require.

If ES6-style modules are actually supported, then removing this property would be spiffy.

Unfortunately (or maybe, fortunately, since allowing multiple ways also seems strange), UXP (for now) only supports CommonJS (and I haven't heard anything about that getting changed any time soon), so at least for now, commonjs is required here.

@ericdrobinson
Copy link
Author

ericdrobinson commented Jan 1, 2020

@pklaschka You raise some excellent points here! I'll start by outlining my recommendation.

Recommendation

Any thoughts?

Yes. Specifically:

  1. Drop jsconfig.json as you suggest.
  2. Do not provide a tsconfig.json file.
  3. Change the "Getting Started" section to simply read something like:

    These Type Declaration files provide your IDE (e.g. Visual Studio Code or WebStorm) with information about the XD API surface, enabling type checking, autocomplete suggestions, and more. To get started, simply run the following command to install the Type Declarations:
    npm install --save-dev @types/adobe-xd@git://github.com/AdobeXD/typings.git
    The Type Declarations should now be available in your project.

  4. Add a subsection to the "Getting Started" section that outlines the following:
    1. Explains that a tsconfig.json (or jsconfig.json if using VSCode) file may be necessary for full autocompletion goodness. This section should provide a description of all of the settings necessary or helpful for UXP development. I would personally call out certain properties as optional and explain when they are to be used (e.g. allowJs and checkJs). An example minimally complete tsconfig.json listing would be good here.
    2. You can then go on to add a sub-section that points users towards resources (i.e. the existing wiki page, IDE manual pages, or tsconfig.json documentation) to help them with further setup for their project.

Remember, the goal of a core repository like this isn't to provide users with a fully optimized solution out of the box, but to provide them with a basic feature set and notes about requirements that they can configure in their own project to match their needs and setup. The more options you configure, the more confusing it is to untangle the assumptions that the setup is making.

For instance, the suggestion I outlined above already assumes the user uses npm (or some equivalent like yarn). You could support a user who doesn't use a package manager by providing instructions on using the Type Declaration files directly, but I would then suggest that you break it out into a separate section or even a separate wiki page (linked in the ReadMe of course) so as to keep things as simple as possible.

How simple? The setup I outlined above allowed me to npm init a new project, install the types, and then begin using the types immediately in both TypeScript and JavaScript contexts in Visual Studio Code. I didn't install/write a tsconfig.json or jsconfig.json file - I just started writing. With no further configuration, Visual Studio Code was able to properly resolve the XDSelection type and properly handle "complex" edge cases like the RootNode type. I was able to copy-paste the contents of sample.js with no type errors (and working type completion). I was able to convert sample.js to a TypeScript equivalent by copying the contents of sample.js into a sample.ts file and work through all the type errors you would expect.

When I did add both a tsconfig.json file and a jsconfig.json file, they looked like the following:

tsconfig.json:

{
    "compilerOptions": {
        "strict": true,
        "target": "es2015",
        "module": "CommonJS",
        "lib": [
            "es2015",
            "dom"
        ]
    }
}

jsconfig.json:

{
    "compilerOptions": {
        "strict": true,
        "target": "es2015",
        "module": "CommonJS",
        "checkJs": true,
        "lib": [
            "es2015",
            "dom"
        ]
    },
  }

Super simple. The only change on the JavaScript side that I noticed was that VSCode's autocomplete stopped suggesting keywords from the surrounding context (as the context was now made explicit).

Here is a screenshot of the project I tossed together to showcase this:

image

TypeScript to the left; JavaScript to the right. You will notice that there is no root-level config file: both src-ts and src-js contain their respective config files. This means that this workspace includes two "projects" in the eyes of the TypeScript language service: one rooted at src-ts and one at src-js, each with their own custom settings.

I then installed TypeScript (the language service was using the version of TypeScript installed with VSCode itself) and ran that TypeScript compiler on my main.ts file and received the expected CommonJS style output with no errors or warnings.

It's hard to think of a simpler way to handle this. As a side benefit, when a strategy for publishing the types is determined, updating the ReadMe file will be as simple as fixing the one npm install line.

Opinionated Setups

None of this stops you from creating (and even linking to) a separate opinionated repository that does include such a fully optimized solution, of course. Systems like your xd-plugin-boilerplate are excellent in that they provide shortcuts that make many decisions for the user already. The key here is keeping them separate: allow the community to build helpful tools that consume this simple repository, rather than try to be everything to everyone.

Remember: each line you add to a config file (in this case tsconfig.json or jsconfig.json) is a decision that you're making about the user's project setup. In this case, less is more: the less you specify, the more users you support.

What About the [t/j]sconfig.json Files in This Repository?

I strongly suggest removing them from the root entirely. If you need a tsconfig.json file for writing/maintaining the types themselves, then put it into the types directory next to the declarations themselves. This will help avoid confusion for consumers of the types.

If you wish to provide sample config files, then I would suggest creating a samples directory with the following structure:

  • ReadMe.md - a file that outlines how to try out the sample(s).
  • javascript - a folder that contains sample.js and a tsconfig.json file configured to work with JavaScript (use tsconfig.json for maximum IDE compatibility).
  • [Optional] typescript - a folder that contains sample.ts and a tsconfig.json file with the minimum settings required for UXP/XD development.

The ReadMe mentioned above would then instruct a user to create a new empty project and copy the contents of one of the sample directories into it. Installing the type declarations (as specified in the main ReadMe) would then allow the system to simply start working.

The Types May Not Resolve In the Samples Directories

With the setup outlined above, the Type Declaration files in the types directory may not resolve when working "inside the repo". This probably depends a lot on the IDE in question. If that's the case, then it might simply be prudent to do one of the following:

  1. Create a separate "Type Declaration Samples" repository (this seems overmuch).
  2. Leave sample.js in the project root and simply remove the config files altogether. While providing a config file is "nice", it may not actually be necessary for the IDE's language service to resolve the types: it may do so simply by looking in neighboring folders.
  3. Use the structure outlined above for samples but add the requisite types/typeRoots properties to allow them to resolve during development. Instruct the user to remove these lines (perhaps with JSON comments).

No Access to Global Interfaces Like XDSelection

In your previous response, you said:

for some reason I haven't found in two hours of debugging, this doesn't grant me access to global interfaces like XDSelection

I honestly have no idea. This was the first thing I tested in VSCode and it worked out-of-the-box with no config file. Some questions:

  1. Which IDE are you testing in?
  2. Do you have a [t/j]sconfig.json file defined?
  3. If so, what are the settings?
  4. Do you know of a way to restart your IDE's language service? (After making adjustments to config files in VSCode you can run the TypeScript: Restart TS server command to force the system to pick up and implement your config file changes...)
  5. Have you tried this in an otherwise empty project?

I hope this helps.

@pklaschka
Copy link
Contributor

And now, XDSelection is working for me, too. I have no idea what I did wrong, yesterday (I've verified via the local history, everything was exactly the same and I had restarted the language service as well), but... So be it 😜

Regarding your suggestions: Sounds good, I'll work on updating the docs in the next few days, so expect to see a PR relatively soon 🙂.

@ericdrobinson
Copy link
Author

As a quick test, I made the organizational adjustments I suggested in the What About the [t/j]sconfig.json Files in This Repository? section of this comment. I found that a tsconfig.json file was necessary in both the types folder and the samples/javascript folder (would also be required for a samples/typescript folder).

I added as few settings as possible to each in order to keep everything simple. What follows is what I came up with:

typings/tsconfig.json

{
    "compilerOptions": {
        "strict": true,
        "lib": [
            "es2015",
            "dom"
        ]
    }
}

Technically, this file isn't even necessary (in Visual Studio Code, at least). The language service relies upon defaults for lib (which appears to be DOM, ES5, and ScriptHost) and has no issues with strict defaulting to false. The configuration above simply makes the expected development environment explicit.

samples/javascript/jsconfig.json

I implemented this as jsconfig.json because it appears that jsconfig.json is recognized by the TypeScript language service itself (even though it's mainly documented in the Visual Studio Code docs). A handful of web searches suggest that WebStorm should be capable of dealing with jsconfig.json files as well, so I went with it.

{
    "compilerOptions": {
        // Enable JavaScript type checking.
        "checkJs": true,

        // Include libraries that XD [mostly] supports.
        "lib": [
            "es2015",
            "dom"
        ],

        // [OPTIONAL] Turn on strict type checking for added safety.
        "strict": true
    },

    // [CUSTOM FOR THIS REPO] Point to the types. These are typically
    //  installed with a package manager. See the repository's root
    //  ReadMe for more.
    "include": [
        // Resolve the type declarations.
        "../../types",
        // Resolve files in this folder and those below.
        "./**/*"
    ]
}

This config uses non-standard JSON comments to add inline explanations for the various settings. The TypeScript tools are capable of dealing with such comments.

jsconfig.json is Standard TypeScript

At time of writing, it sets the following values:

  • "allowJs": true
  • "maxNodeModuleJsDepth": 2
  • "allowSyntheticDefaultImports": true
  • "skipLibCheck": true
  • "noEmit": true

With this setup, the root of the project was much simpler and the purpose of each config file much clearer.

@pklaschka pklaschka self-assigned this Jan 2, 2020
@ericdrobinson
Copy link
Author

@pklaschka While testing something out, I added a typescript directory to the sample structure I outlined in my previous comment.

I'll post their contents here in the event that it's something you're interested in adding to the project:

samples/typescript/tsconfig.json

{
    "compilerOptions": {
        // AdobeXD requires CommonJS style module.
        "module": "commonjs",

        // Include libraries that XD [mostly] supports.
        "lib": [
            "es2015",
            "dom"
        ],

        // [OPTIONAL] Turn on strict type checking for added safety.
        "strict": true
    },

    // [CUSTOM FOR THIS REPO] Point to the types. These are typically
    //  installed with a package manager. See the repository's root
    //  ReadMe for more.
    "include": [
        // Resolve the type declarations.
        "../../types",
        // Resolve files in this folder and those below.
        "./**/*"
    ]
}

As with the jsconfig.json file, this one contains JSON Comments for config clarity.

samples/typescript/sample.ts

import {Text, Ellipse, Color} from "scenegraph";
import * as clipboard from "clipboard";
import {shell} from "uxp";
import * as assets from 'assets';
import * as uxp from 'uxp';

const fs = uxp.storage.localFileSystem;

async function test(selection: XDSelection, documentRoot: import('scenegraph').RootNode) {
    for (const node of selection.items) {
        console.log("Hello world: ", node);
        if (node instanceof Text) {
            clipboard.copyText(node.text);
        } else if (node instanceof Ellipse) {
            node.fill = new Color("#ffaaee");
            await shell.openExternal('https://adobe-xd.gitbook.io/plugin-api-reference/uxp-api-reference/network-apis/shell');
        }
    }
    const tempFolder = await fs.getTemporaryFolder();
    const newFile = await tempFolder.createFile("tempfile.txt", {overwrite: true});
    await newFile.write("Hello, world!");
    await newFile.moveTo(tempFolder, {overwrite: true});

    const anotherFile = await tempFolder.getEntry('tempfile.txt');
    if (anotherFile.isFile) {
        anotherFile.write("Good day");
    } else if (anotherFile.isFolder) {
        console.log("That's a folder. It shouldn't be a folder. What have you done?")
    }

    const colors = assets.colors.get();
    console.log(colors);
}

export const commands = {
    test: test
};

This is a pretty straightforward translation of your sample.js file.

@pklaschka
Copy link
Contributor

Just FYI: After further testing, I can conclude that JetBrains IDEs don't support jsconfig.json files (but do support tsconfig.json files), which is why I'd like to use this (in fact, I don't know of any editor except VSCode that supports the jsconfig.json, and if it's really just a preconfigured version of the tsconfig.json, I see no reason to use the jsconfig.json file at all, decreased compatibility and the same end-result in compatible environments don't give it much of an advantage 🙂 ).

@ericdrobinson
Copy link
Author

ericdrobinson commented Jan 3, 2020

JetBrains IDEs don't support jsconfig.json files

Which IDE are you using? A quick Google search shows that there are lots of people using jsconfig.json with WebStorm. This gist even has a WebStorm-specific section.

[EDIT] I'm not certain the gist I linked outlines how to use the jsconfig.json file with WebStorm - it may simply say "for the equivalent setup in WebStorm, do this". ¯\(ツ)

@ericdrobinson
Copy link
Author

ericdrobinson commented Jan 3, 2020

The WebStorm 2019.2 release announcement even mentions jsconfig.json...

@ericdrobinson
Copy link
Author

@pklaschka

I don't know of any editor except VSCode that supports the jsconfig.json

My understanding is that any IDE that relies upon the TypeScript language service to handle JavaScript type checking will support the jsconfig.json file. This resource explains how to configure jsconfig.json for Atom. It also mentions that jsconfig.json doesn't work for WebStorm (at the time of writing), but that was dated Feb. 2018. The 2019.2 release of WebStorm, at least, seems as though it should support it.

The JetBrains folks do their own magic for type checking JavaScript, using the [ts/js]config.json files for hints.

At the end of the day, I don't really care if you use jsconfig.json or tsconfig.json. I personally believe that using jsconfig.json makes your intent more clear and simplifies the config file a whole bunch (no need for "noEmit": true for instance). If it feels like I'm pushing for jsconfig.json, it's due to a preference for the more simple solution.

For clarity, which JetBrains IDE (including version number) are you using?

@pklaschka
Copy link
Contributor

@ericdrobinson The thing is that (to my knowledge, I don't have the time to find sources for this statement right now) Webstorm, unlike VSCode?, doesn't use TypeScript as a language server by default. When I add the tsconfig.json file, it begins to additionally also use a TypeScript language service for checking the files (which is when it also begins to recognize the import().ABC syntax in JSDoc comments etc.), but it doesn't use this service with a jsconfig.json.

I'm using WebStorm (latest version) as an IDE.

@pklaschka
Copy link
Contributor

It is actually the "allowJS": true in the tsconfig.json that enables the Typescript Language Service in WebStorm, according to https://blog.jetbrains.com/webstorm/2019/09/using-typescript-to-check-your-javascript-code/:

Now for the most important step: we need to add the option "allowJs": true to the compilerOptions property in tsconfig.json. This will allow TypeScript to check your JavaScript files.

[...]

To check all files, we need to add the "checkJs": true option to tsconfig.json – this will enable TypeScript for all JavaScript files in your project.

[...]

As you may know, the TypeScript language service can also provide you with code completion suggestions and fixes for the errors it finds.

Similar to how WebStorm uses these suggestions to improve its code completion in the TypeScript files, with the "allowJs": true option you’ll get these suggestions in JavaScript files too. They will be merged together with WebStorm’s own suggestions.

@ericdrobinson
Copy link
Author

it doesn't use this service with a jsconfig.json.

Does the jsconfig.json file you're testing with have the "checkJs": true option set?

@pklaschka
Copy link
Contributor

pklaschka commented Jan 3, 2020

@ericdrobinson Yes:
image

(it's basically my working tsconfig.json without allowJS...)

@ericdrobinson
Copy link
Author

Well it's really up to you. Perhaps you add a tsconfig.json and, if the JetBrains folks get around to implementing proper support for jsconfig.json, then it can change.

Perhaps it makes sense to file a bug with WebStorm to have it pick up on JavaScript type checking when a jsconfig.json file exists?

@pklaschka
Copy link
Contributor

@ericdrobinson

Well it's really up to you. Perhaps you add a tsconfig.json and, if the JetBrains folks get around to implementing proper support for jsconfig.json, then it can change.

Sounds good.

Perhaps it makes sense to file a bug with WebStorm to have it pick up on JavaScript type checking when a jsconfig.json file exists?

When I find the time to properly confirm this behavior and write a bug report, I'll probably do that. As the exams at my university currently (once again) are getting closer and closer at an alarming rate, that'll still have to wait a bit, though... 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants