-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
5.3.2 adds broken "reference" comment #56571
Comments
I've noticed this, too:
that whole reference line should not be present.
the main thing through, is that as a library author, I've declared AS ISthe triple slash reference behavior lends itself to libraries "breaking" during patch releases because typescript is exposing stuff it shouldn't. Work-aroundAfter generating declarations, remove triple slash directives grep -r -l '///' declarations/ | sort | uniq | xargs perl -e \"s/\\/\\/\\/.*$//\" -pi |
Likely caused by #55636 |
I think you’re probably using |
@andrewbranch my tsconfig settings are already strict, and I don't use tsc with |
@NullVoxPopuli I was addressing that comment to @rchl. Your case looks like a bug where the generated path isn’t respecting
The reference is needed. Knowing the consumers will have |
Looks like you are right. I have
which looks like should work fine (haven't tested to consume that yet). |
Though I wonder if Typescript could do a "better" job when |
It's an incorrect reference though -- TS made the wrong call. |
@NullVoxPopuli again, the path is incorrect, indicating a bug, but the reference is necessary. If there’s no way to write a path to the globals you’re implicitly referencing during your own compilation of the library, we should be giving you an error during declaration emit. If you have a repro for this, please open a new issue. @rchl yeah, I think we could explore skipping paths/baseUrl for these paths that get generated in TypeScript-only constructs like triple-slash references. But that kind of hides the underlying misconfiguration here—the reality is that you could have written |
Just an extra note that TS is adding: /// <reference path="../../module/module.d.ts" /> even though there also exists a related type import, importing from the same file: import type { ModuleOptions } from '../../module/module'; So in that case at least the reference looks redundant. While I don't disagree that using |
Hm, that import does make the triple-slash reference redundant. I would consider it an improvement if we could get rid of it, but not sure I’d classify its presence as a bug, or at least not a high priority one.
Well, that’s exactly the issue—the triple-slash reference didn’t appear because of your import; on the contrary, it appeared because we failed to notice you already had an import resolving to the same thing, making the reference redundant. The triple-slash references appear when you access a global (or ambient module declaration, or export of a module augmentation). So we’re not taking the path you wrote and mangling it; we’re generating a path from scratch. If you can make a minimal repro of this happening, I can at least open it up to community PRs. |
Can we have an option in compilerOptions for declaration emitting to not ever emit triple slash directives? |
No, we don’t add options that intentionally break declaration emit 🤨 |
@andrewbranch if you want to investigate my case, even though it's technically invalid, I've created this minimal repro: https://github.com/rchl/ts-reference-test (includes compiled |
Just so everyone has all the relevant context (and we should be super clear that the emit issue here is almost certainly something wacky we are doing on the Ember side, not a TS bug!)— The mechanics of how Ember’s stable types are currently published are (as has been very typical for us) doing horrible things to deal with Ember’s “We implemented ESM ourselves!” decision back in 2015–2016, which in turn our type publishing mechanics have to account for and try to “translate” to something resolveable by TS (ultimately: by the standard Node resolution algorithm). The key constraint here was: Ember users import from specifiers like To enable a phased transition out of DefinitelyTyped, we took a two-step process of providing first “preview” and then “stable” types. In each case, users get access to the types by writing a side-effect-style import: import 'ember-source/types/preview'; or import 'ember-source/types'; The “preview” types were copied over from DefinitelyTyped, and written as a set of We also began publishing types from source incrementally, and so recommended that libraries and apps use this as the way of getting both simultaneously: import 'ember-source/types/preview';
import 'ember-source/types'; Libraries depend on The contents of the Stable types are published from Ember’s own source, using a custom build script which:
This is a very weird way to have to make things visible, and it is possible that it is not playing nicely with how It is not clear to me how or why any specific one of those would be at issue, and as I am no longer actively contributing to Ember or its types, I will not be able to dig in further, but I am happy to answer questions, provide further info, etc. here to help as I can. Since they are not available in the repo but exist only as part of the build: The generated
|
I was under the impression that those triple-slash paths don’t resolve at all because they were ignoring
"types": [
"types/stable"
],
"types/preview": [
"types/preview"
],
+ "types/*": [],
+ "types/preview/*": [] I agree, there is no TypeScript bug here. I opened #56614 to address #56571 (comment). Closing this out. |
Made a tool to help out for my use case
I guess it's my opinion 🤷♂️ , but anytime a it is expected that the consumer of your library will provide the types |
Now more generically, https://github.com/NullVoxPopuli/fix-bad-declaration-output will remove all And as an inline rollup plugin: plugins: [
{
name: "generate declarations",
closeBundle: async () => {
await execaCommand('tsc --declaration', { stdio: 'inherit' });
}
},
{
name: 'fix declaration output',
closeBundle: async () => {
await fixBadDeclarationOutput(
`declarations/**/*.d.ts`,
['TypeScript#56571']
);
}
}
] |
…ipt#56571 issue Normalize some checking, expand test cases
As of #57681, TS will no longer synthesize reference directives, nor preserve reference directives unless explicitly annotated with |
🔎 Search Terms
reference types error
🕗 Version & Regression Information
⏯ Playground Link
No response
💻 Code
My code does a type import like:
and in 5.3.2 this adds a type reference in the compiled output:
/// <reference types="dist/module/module" />
which is an invalid path from the point of the user of this package and result in an error:
🙁 Actual behavior
Adds a broken reference:
/// <reference types="dist/module/module" />
🙂 Expected behavior
5.2.2 didn't add a reference in this case.
Additional information about the issue
No response
The text was updated successfully, but these errors were encountered: