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

Superfluous triple-slash directives generated by Glint produce Glint errors (manually removing them fixes this) #564

Open
fry69 opened this issue Feb 8, 2024 · 8 comments · May be fixed by #565

Comments

@fry69
Copy link

fry69 commented Feb 8, 2024

In a fresh 5.6.0 install (ember new app --typescript) with just ember-concurrency 4.0.0 and most recent Glint (ember install ember-concurrency and following the basic instructions from https://typed-ember.gitbook.io/glint/environments/ember/installation), I get these errors when I run Glint on the terminal:

$ ./node_modules/.bin/glint
node_modules/ember-concurrency/declarations/helpers/cancel-all.d.ts:1:23 - error TS2688: Cannot find type definition file for 'ember-source/types/preview/@ember/component/-private/signature-utils'.

1 /// <reference types="ember-source/types/preview/@ember/component/-private/signature-utils" />
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/ember-concurrency/declarations/helpers/cancel-all.d.ts:2:23 - error TS2688: Cannot find type definition file for 'ember-source/types/preview/@ember/component/helper'.

2 /// <reference types="ember-source/types/preview/@ember/component/helper" />
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/ember-concurrency/declarations/helpers/perform.d.ts:1:23 - error TS2688: Cannot find type definition file for 'ember-source/types/preview/@ember/component/helper'.

1 /// <reference types="ember-source/types/preview/@ember/component/helper" />
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/ember-concurrency/declarations/helpers/task.d.ts:1:23 - error TS2688: Cannot find type definition file for 'ember-source/types/preview/@ember/component/-private/signature-utils'.

1 /// <reference types="ember-source/types/preview/@ember/component/-private/signature-utils" />
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/ember-concurrency/declarations/helpers/task.d.ts:2:23 - error TS2688: Cannot find type definition file for 'ember-source/types/preview/@ember/component/helper'.

2 /// <reference types="ember-source/types/preview/@ember/component/helper" />

There are no preview types in ember-source for 5.6.0?

$ ls node_modules/ember-source/types
publish.mjs	stable

Looks like ember-page-title has had a similar issue with generated triple-slash directives, see ember-cli/ember-page-title#283

I can confirm that manually removing these generated /// lines fixes this issue for me.

@fry69 fry69 changed the title Glint errors / ember-source preview types? Superfluous generated triple-slash directives generated by Glint produce Glint errors (manually removing them fixes this) Feb 8, 2024
@fry69 fry69 changed the title Superfluous generated triple-slash directives generated by Glint produce Glint errors (manually removing them fixes this) Superfluous triple-slash directives generated by Glint produce Glint errors (manually removing them fixes this) Feb 8, 2024
@machty
Copy link
Owner

machty commented Feb 9, 2024

@NullVoxPopuli can you confirm that applying the same solution/codefix from ember-cli/ember-page-title#283 would be the proper course of action here?

@NullVoxPopuli
Copy link

NullVoxPopuli commented Feb 9, 2024

Can confirm.
These folks have used that solution so far: https://github.com/NullVoxPopuli/fix-bad-declaration-output/network/dependents

Gonna add it to the ts blueprint for v2 addons

@machty machty linked a pull request Feb 9, 2024 that will close this issue
@fry69
Copy link
Author

fry69 commented Feb 9, 2024

Until the fix has landed and file in the node-modules directory tend to get replaced to their original state, here is a python script to remove those triple-slash lines.

import os
import re

dir_path = './node_modules/ember-concurrency/declarations/helpers'

for file_name in os.listdir(dir_path):
    if file_name.endswith('.d.ts'):
        file_path = os.path.join(dir_path, file_name)
        
        with open(file_path, 'r') as f:
            file_contents = f.read()
        
        file_contents = re.sub('^/// <reference types="[^"]+" />', '', file_contents, flags=re.MULTILINE)
        
        with open(file_path, 'w') as f:
            f.write(file_contents)

@NullVoxPopuli
Copy link

NullVoxPopuli commented Feb 9, 2024

Was digging in to this yesterday, and it looks like the generated declarations may slightly be wrong? They're not using declare when exporting a const, and that breaks the ts parser used in fix-bad-declaration-output

@dfreeman
Copy link

dfreeman commented Feb 27, 2024

Should we be using generated declarations at all? Last I knew ember-concurrency was still using a hand-rolled index.d.ts and the bulk of the actual implementation was still in JS, so I wonder if things were unintentionally switched over as part of the v2 migration.

Edit: ah, I was getting trolled by the start:types script not having the extra copy step cp src/index.d.ts declarations.d.ts step that the build:types script has.

@esbanarango
Copy link

esbanarango commented Aug 9, 2024

Any updates on this one?

I'm using:

"ember-source": "5.10.2",
"ember-concurrency": "4.0.2",

and I'm getting:

node_modules/ember-concurrency/declarations/helpers/perform.d.ts:1:23 - error TS2688: Cannot find type definition file for 'ember-source/types/prev…
│ [types] 
│ [types] 1 /// <reference types="ember-source/types/preview/@ember/component/helper" />

@NullVoxPopuli
Copy link

ember-concurrency has a bug in their types, and they may need to add this inline-rollup plugin: https://github.com/universal-ember/ember-primitives/blob/main/ember-primitives/rollup.config.mjs#L34

 {
      name: "Build Declarations",
      closeBundle: async () => {
        /**
         * Generate the types (these include /// <reference types="ember-source/types"
         * but our consumers may not be using those, or have a new enough ember-source that provides them.
         */
        console.log("Building types");
        await execaCommand(`pnpm glint --declaration`, { stdio: "inherit" });
        /**
         * https://github.com/microsoft/TypeScript/issues/56571#
         * README: https://github.com/NullVoxPopuli/fix-bad-declaration-output
         */
        console.log("Fixing types");
        await fixBadDeclarationOutput("declarations/**/*.d.ts", [
          ["TypeScript#56571", { types: "all" }],
          "Glint#628",
          "Glint#697",
        ]);
        console.log("⚠️ Dangerously (but neededly) fixed bad declaration output from typescript");
      },

I ultimately consider this a bug with TypeScript itself though, because they don't have a story for peer-provided types at all.

@ef4
Copy link
Contributor

ef4 commented Sep 23, 2024

Can confirm that to user ember-concurrency 4.0.2 with a new-enough ember-source (which lacks types/preview since they're not preview anymore) I had to patch:

diff --git a/declarations/helpers/perform.d.ts b/declarations/helpers/perform.d.ts
index 140c903b9428c57f1de4c66e3a4143a77742f5ea..989e429ba29ec126ce603b0cb0f132339094b2ce 100644
--- a/declarations/helpers/perform.d.ts
+++ b/declarations/helpers/perform.d.ts
@@ -1,4 +1,3 @@
-/// <reference types="ember-source/types/preview/@ember/component/helper" />
 import type { Task } from '../index';
 type PerformParams = [task: Task<any, any[]>, ...args: any[]];
 export declare function performHelper(args: PerformParams, hash: any): (...innerArgs: any[]) => any;

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

Successfully merging a pull request may close this issue.

6 participants