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

Configuration API (UserConfig) types, TypeScript mega issue for 3.0 #3097

Closed
zachleat opened this issue Nov 10, 2023 · 23 comments · Fixed by #3349
Closed

Configuration API (UserConfig) types, TypeScript mega issue for 3.0 #3097

zachleat opened this issue Nov 10, 2023 · 23 comments · Fixed by #3349
Assignees
Labels
bug typescript Type definitions and Typescript issues

Comments

@zachleat
Copy link
Member

zachleat commented Nov 10, 2023

(This one is just a reminder for me)

See: https://www.11ty.dev/docs/config/#type-definitions

Pre-alpha I can confirm that v9 (ESM) of eleventy-base-blog with npm installed via file:../eleventy works fine but I want to remind myself to check post-publish too.

@zachleat zachleat added the typescript Type definitions and Typescript issues label Nov 10, 2023
@zachleat zachleat added this to the Eleventy 3.0.0 milestone Nov 10, 2023
@rubenwardy
Copy link

This doesn't work for me:

import { UserConfig } from "@11ty/eleventy";

I need to do:

import UserConfig from "@11ty/eleventy/src/UserConfig.js";

[11ty] 1. Error in your Eleventy config file '.eleventy.js'. (via EleventyConfigError)
[11ty] 2. The requested module '@11ty/eleventy' does not provide an export named 'UserConfig' (via SyntaxError)
[11ty] 
[11ty] Original error stack trace: file:///home/ruben/dev/web/blog/plugins/wordcount.js:1
[11ty] import {UserConfig} from "@11ty/eleventy";
[11ty]         ^^^^^^^^^^
[11ty] SyntaxError: The requested module '@11ty/eleventy' does not provide an export named 'UserConfig'
[11ty]     at ModuleJob._instantiate (node:internal/modules/esm/module_job:124:21)
[11ty]     at async ModuleJob.run (node:internal/modules/esm/module_job:190:5)

@rubenwardy
Copy link

see also #3127

@uncenter
Copy link
Contributor

uncenter commented Dec 19, 2023

#3060 removed the .d.ts file to rely on JSDoc entirely, but the .d.ts had been exporting UserConfig (https://github.com/11ty/eleventy/blob/ecd0579a2dded6939510b7c23841388b26eb6d16/src/index.d.ts) and the issue now that is UserConfig isn't exported anywhere else. Would the fix be to export it here?

@what1s1ove
Copy link

#3060 removed the .d.ts file to rely on JSDoc entirely, but the .d.ts had been exporting UserConfig (https://github.com/11ty/eleventy/blob/ecd0579a2dded6939510b7c23841388b26eb6d16/src/index.d.ts) and the issue now that is UserConfig isn't exported anywhere else. Would the fix be to export it here?

Agree. I have tsconfig which checks all JS files (including eleventy.config.js). Now I need to manually describe all the config keys 😕

@uncenter
Copy link
Contributor

#3060 removed the .d.ts file to rely on JSDoc entirely, but the .d.ts had been exporting UserConfig (ecd0579/src/index.d.ts) and the issue now that is UserConfig isn't exported anywhere else. Would the fix be to export it here?

Agree. I have tsconfig which checks all JS files (including eleventy.config.js). Now I need to manually describe all the config keys 😕

You don't have to do that, can't you just use it like /** @param { import("@11ty/eleventy/src/UserConfig.js") } eleventyConfig */ (as suggested here)?

@what1s1ove
Copy link

what1s1ove commented Dec 23, 2023

#3060 removed the .d.ts file to rely on JSDoc entirely, but the .d.ts had been exporting UserConfig (ecd0579/src/index.d.ts) and the issue now that is UserConfig isn't exported anywhere else. Would the fix be to export it here?

Agree. I have tsconfig which checks all JS files (including eleventy.config.js). Now I need to manually describe all the config keys 😕

You don't have to do that, can't you just use it like /** @param { import("@11ty/eleventy/src/UserConfig.js") } eleventyConfig */ (as suggested here)?

Unfortunately, no...

Today I migrated to a new version. I'm so happy that the number of cjs files is decreasing in my projects 🎉
what1s1ove/whatislove.dev#279

After migration, I encountered two issues.

  1. "Replace await readFile(pathTojson) with import attributes (Node.js versions 18 and 20 are different!)". Discussed in this issue - Import attributes not yet supported (Error thrown from acorn dependency) #3128
  2. "Replace custom type definitions for the 11ty package" – relates to this issue.

As I mentioned earlier, my project is configured in a way that TypeScript performs checks on all .js files (in fact, not only TypeScript but many other linters do the same). Before migrating the 11ty config to js, it was ignored because it had the .cjs extension.

When TypeScript encounters an import of a package that has no types, it suggests two solutions:

  1. Install types separately. In our case, this is not possible, as there are no types for 11ty as a separate package (and there's no need to have them as a separate package 🤞).
  2. Create custom types using declare module. However, the problem with declare module is that within this construct, you cannot import packages that don't have types (well, you can, but any such import would result in any type). It means that the @11ty/eleventy/src/UserConfig.js import cannot be used. Because of this, all types should be defined manually.
image

I think before migrating to ESM, this was a rare problem, as I mentioned, many linters ignored .cjs by default. Because of this, the 11ty config didn't fall under various linters (eslint, TypeScript, etc). But now, since this will be the default behavior, this problem will affect many users. It seems to me that it should be addressed by adding d.ts files to the 11ty package itself.

@murtuzaalisurti
Copy link

murtuzaalisurti commented Mar 29, 2024

While migrating to 11tyv3.0.0-alpha.5, the JSDoc comments stopped working for me. It seems that in ESM, you have to explicitly specify the default export.

/** @param { import("@11ty/eleventy").UserConfig } eleventyConfig */ 

/** @param { (import("@11ty/eleventy/src/UserConfig").default) } eleventyConfig */ 

this works fine as well:

/**
 * @typedef {import('@11ty/eleventy/src/UserConfig').default} EleventyConfig
 * @typedef {import('@11ty/eleventy/src/defaultConfig').defaultConfig} EleventyReturnValue
 * @type {(eleventyConfig: EleventyConfig) => EleventyReturnValue}
 */

@zachleat
Copy link
Member Author

See #3291

@zachleat
Copy link
Member Author

/** @param {import("@11ty/eleventy").UserConfig} eleventyConfig */ seems to work with 3.0.0-alpha.12 (shipped by #3291).

Closing this but would appreciate further confirmation!

@what1s1ove
Copy link

what1s1ove commented Jun 12, 2024

Hey guys!

Does this work for anyone? It finds the import, but it is marked as any. Because of this, there are no suggestions for the config methods/properties.

image

@zachleat
Copy link
Member Author

zachleat commented Jul 5, 2024

@mayank99 helped out here with #3349, shipping with 3.0.0-alpha.15

@what1s1ove
Copy link

what1s1ove commented Jul 6, 2024

@mayank99 helped out here with #3349, shipping with 3.0.0-alpha.15

Hey @zachleat ! Could you let me know when you're going to publish it? 🙏

@tbroyer
Copy link

tbroyer commented Jul 10, 2024

I have @param {import("@11ty/eleventy").UserConfig} eleventyConfig in JSDoc that works with 3.0.0-alpha.14 but fails with 3.0.0-alpha.16 with the same error as in #3097 (comment) when using tsc to lint my JS code.

.eleventy.js:22:19 - error TS7016: Could not find a declaration file for module '@11ty/eleventy'. '…/node_modules/@11ty/eleventy/src/Eleventy.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/11ty__eleventy` if it exists or add a new declaration (.d.ts) file containing `declare module '@11ty/eleventy';`

22  * @param {import("@11ty/eleventy").UserConfig} eleventyConfig
                     ~~~~~~~~~~~~~~~~

@mayank99
Copy link
Sponsor Contributor

mayank99 commented Jul 10, 2024

@tbroyer (and @what1s1ove), I just tested 3.0.0-alpha.16 and this works totally fine:

/** @param {import("@11ty/eleventy").UserConfig} eleventyConfig */
export default function (eleventyConfig) {

Here's a minimal stackblitz link to confirm: https://stackblitz.com/edit/github-7xpr1h-tbkpjx?file=.eleventy.js

Maybe you can share a minimal repro if it's not working for you? If you have a custom tsconfig, it's possible that that might be interfering with the default types resolution.

@tbroyer
Copy link

tbroyer commented Jul 11, 2024

This stackblitz actually exhibits that same error:
image

@mayank99
Copy link
Sponsor Contributor

mayank99 commented Jul 11, 2024

I guess I don't understand the problem with that. The eleventyConfig object is still typed, and the autocomplete still works fine.

@tbroyer
Copy link

tbroyer commented Jul 11, 2024

Got it: this is tsc's --strict, that generates an error out of this: https://stackblitz.com/edit/github-7xpr1h-crqrvo?file=.eleventy.js,package.json

Because 3.0.0-alpha.14 had a "type" entry in its package.json pointing to a .d.ts, this worked great. We already had to add @ts-ignore to a couple imports like I did with @11ty/eleventy-plugin-webc here, but at least it worked with @11ty/eleventy.

Should a .d.ts be generated out of the JSDoc comments? (and added back to the package.json)

@zachleat zachleat reopened this Jul 11, 2024
@zachleat zachleat self-assigned this Jul 25, 2024
@zachleat
Copy link
Member Author

Spent some time looking into this and found a few things worth noting:

v3.0.0-alpha.14 failed tsc --strict with the following error:

~/Temp/eleventy-3097-ts-strict ᐅ npx tsc --noEmit --checkJs --allowJS --strict .eleventy.js
node_modules/@11ty/eleventy/src/index.d.ts:1:24 - error TS7016: Could not find a declaration file for module './UserConfig'. '/Users/zachleat/Temp/eleventy-3097-ts-strict/node_modules/@11ty/eleventy/src/UserConfig.js' implicitly has an 'any' type.

1 import UserConfig from "./UserConfig";
                         ~~~~~~~~~~~~~~


Found 1 error in node_modules/@11ty/eleventy/src/index.d.ts:1

In v3.0.0-alpha.14 tsc worked without --strict.

Irrespective of --strict, the "viral" nature of TypeScript requires the Eleventy codebase to obey TypeScript conventions for any types we want to export. I attempted to use the noResolve to work around this and only provide types for Eleventy and UserConfig (skipping a large refactor) but this caused other issues with node types: https://fediverse.zachleat.com/@zachleat/112848095569188008

The @typedef approach we swapped to here seems to be causing issues with tsc because it runs through Eleventy.js and checks it and all of Eleventy.js’ dependencies (lots of errors) on the way to UserConfig.js.

The previous approach worked better only because it exported UserConfig directly (not via the Eleventy object), e.g. index.d.ts:

import UserConfig from "./src/UserConfig.js";

export { UserConfig };

That said, I think it’d be useful to have IDE autocomplete and tsc compatibility for Eleventy.js and UserConfig.js but for the short term I’m going to refactor the deps of UserConfig.js to see if we can make some progress here that makes everyone happy and makes the Configuration API more usable.

I’ve filed the Eleventy.js work at #3383.

@zachleat
Copy link
Member Author

Alright I have a bit more clarity on this now.

The previous index.d.ts file we had on this project would not and did not pass a tsc --strict check because the project does not and did not include first-party .d.ts files. The index.d.ts file we had pointed to a JavaScript file and no other .d.ts files existed. From my testing it looks like tsc --strict throws an error if a module’s .d.ts file cannot be found.

node_modules/@11ty/eleventy/src/index.d.ts:1:24 - error TS7016: Could not find a declaration file for module './UserConfig.js'. '/Users/zachleat/Temp/eleventy-3097-ts-strict/node_modules/@11ty/eleventy/src/UserConfig.js' implicitly has an 'any' type.

1 import UserConfig from "./UserConfig.js";
                         ~~~~~~~~~~~~~~~~~


Found 1 error in node_modules/@11ty/eleventy/src/index.d.ts:1

The good part of this exploration is that I’m well educated on the topic now and I’m not opposed to including .d.ts files on the project (at some point), but we’ll need to work through and fix the remainder of the TypeScript slop errors first. I’ve finished the ones for UserConfig.js at #3384 but I’ll want to finish the rest of them (see #3383) before we ship .d.ts files with the project (and that won’t be in time for 3.0, full disclosure).

We’ll keep the existing @typedef approach in place as it accommodates IDE autocomplete use cases that folks are already using. In my testing, restoring the previous index.d.ts did not offer any additional benefit for IDE autocomplete over the @jsdoc approach we already have in place (nor did it pass tsc --strict as already noted).

So, TL;DR I do plan to merge #3384 in service of the long term goal of shipping .d.ts type files with Eleventy (#3383), but .d.ts files will not ship as part of 3.0.

We’ll continue to recommend folks use https://www.11ty.dev/docs/config/#type-definitions for IDE autocomplete.

If I’ve missed something important here, please let me know!

@uncenter
Copy link
Contributor

Would you ever consider writing Eleventy in TS itself? And then compiling down to JS with generated type definition files?

@zachleat
Copy link
Member Author

With a shout out to the approach @panoply is pushing for with #3296 that seems to be a bit cleaner pattern for configuration moving forward using a defineConfig export instead of a /** @param */ jsdoc comment.

@zachleat
Copy link
Member Author

@uncenter No, probably not. TypeScript can export .d.ts files from JSDoc which is the method I’d prefer moving forward here, as TypeScript becomes a progressive enhancement on top of the JavaScript code base (rather than the other way around, which has a shorter shelf life and more maintenance risk). Happy to revisit a larger discussion about types if and when they’ve shipped as part of JavaScript though!

@zachleat
Copy link
Member Author

Just some clarification on the history here, since it’s been a bit all over the place and it’s worth documenting at the very least for future me (sorry folks, I’m more educated on TypeScript now 😭).

@zachleat zachleat changed the title 3.0 test userconfig types export ok Configuration API (UserConfig) types, TypeScript mega issue for 3.0 Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug typescript Type definitions and Typescript issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants