eleventy icon indicating copy to clipboard operation
eleventy copied to clipboard

3.0 test userconfig types export ok

Open zachleat opened this issue 2 years ago • 7 comments
trafficstars

(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 avatar Nov 10 '23 22:11 zachleat

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 avatar Dec 19 '23 12:12 rubenwardy

see also https://github.com/11ty/eleventy/issues/3127

rubenwardy avatar Dec 19 '23 12:12 rubenwardy

#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?

uncenter avatar Dec 19 '23 12:12 uncenter

#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 😕

what1s1ove avatar Dec 23 '23 15:12 what1s1ove

#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)?

uncenter avatar Dec 23 '23 15:12 uncenter

#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 🎉 https://github.com/what1s1ove/whatislove.dev/pull/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 - https://github.com/11ty/eleventy/issues/3128
    • https://nodejs.org/docs/latest-v18.x/api/esm.html#import-attributes
    • https://nodejs.org/docs/latest-v20.x/api/esm.html#import-attributes
  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.

what1s1ove avatar Dec 23 '23 16:12 what1s1ove

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}
 */

murtuzaalisurti avatar Mar 29 '24 16:03 murtuzaalisurti

See #3291

zachleat avatar Jun 10 '24 21:06 zachleat

/** @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!

zachleat avatar Jun 11 '24 21:06 zachleat

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

what1s1ove avatar Jun 12 '24 05:06 what1s1ove

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

zachleat avatar Jul 05 '24 16:07 zachleat

@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? 🙏

what1s1ove avatar Jul 06 '24 14:07 what1s1ove

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 https://github.com/11ty/eleventy/issues/3097#issuecomment-1868322883 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
                     ~~~~~~~~~~~~~~~~

tbroyer avatar Jul 10 '24 07:07 tbroyer

@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.

mayank99 avatar Jul 10 '24 21:07 mayank99

This stackblitz actually exhibits that same error: image

tbroyer avatar Jul 11 '24 11:07 tbroyer

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

mayank99 avatar Jul 11 '24 12:07 mayank99

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)

tbroyer avatar Jul 11 '24 12:07 tbroyer

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 avatar Jul 26 '24 22:07 zachleat

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!

zachleat avatar Jul 29 '24 20:07 zachleat

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

uncenter avatar Jul 29 '24 20:07 uncenter

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 avatar Jul 29 '24 20:07 zachleat

@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 avatar Jul 29 '24 20:07 zachleat

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 😭).

  • index.d.ts was originally added in 2.0.0-canary.19 via #2091 https://github.com/11ty/eleventy/blob/v2.0.0-canary.19/package.json
  • it was removed prior to the first 3.0 release 3.0.0-alpha.1 https://github.com/11ty/eleventy/pull/3060
  • it was re-added in 3.0.0-alpha.11 https://github.com/11ty/eleventy/pull/3291
  • it was removed again in 3.0.0-alpha.15 https://github.com/11ty/eleventy/pull/3349

zachleat avatar Jul 29 '24 21:07 zachleat