-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Module resolution via "--paths" fails against declaration files generated alongside "--outfile:" #18311
Comments
can you share a repo i can look at, the tsconfig.json seem to be missing |
I've created a repo here: https://github.com/josh-sachs/typescript-18311 |
there is not a module in file // bundles\broken.d.ts
export * from "resource-1"; // bundles\resource-1.d.ts
export class Resource1 {
} |
I'm not sure I follow... All of the exports are declared as modules within the declaration files. The declaration files are generated by TSC via the compiler options:
Typescript successfully resolves the declaration files If I were not using the In essence the declaration file produce by TSC in conjunction with the |
@mhegazy I think you were too quick to label this thread as a question. Please review again as there is an issue here with module resolution against declaration files generated via --outFile. |
A module is a file that has at least one top-level An ambient module declaration, something of the form Now, for module resolution, the target of an import needs to be a module. the compiler asserts that. in the case of your
|
more info about module can be found in http://www.typescriptlang.org/docs/handbook/modules.html |
With all due respect, it's starting to feel like you aren't taking the time to evaluate the information I've put together.
The declaration file declares modules.
Typescript resolves the modules, but not using the proper resolution strategy.
I've put together extremely clear repro steps to demonstrate exactly where the resolution strategy is breaking down.
Please take a moment to review.
…________________________________
From: Mohamed Hegazy <[email protected]>
Sent: Monday, September 11, 2017 1:31:13 PM
To: Microsoft/TypeScript
Cc: Joshua Sachs; Author
Subject: Re: [Microsoft/TypeScript] Module resolution via "--paths" fails against declaration files generated alongside "--outfile:" (#18311)
more info about module can be found in http://www.typescriptlang.org/docs/handbook/modules.html
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#18311 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ALFAbUlgZK3Flo0z7CUoUSJaFcmzkcSmks5shW5hgaJpZM4PP7kc>.
|
This file does declare modules, one called declaring a global module called declare module "resource-1" {
export class Resource1 {
}
}
declare module "index" {
export * from "resource-1";
}
Not sure i understand what you mean. |
Typescript allows import statements against the definition file. If you get the repo and open
the path Typescript correctly resolves Now notice the following:
The reason that the
The reason that the
What is expected to happen is that TSC should automatically find the index module when when importing
Typescript already resolves barrels this way when --outFile is not present... from: https://www.typescriptlang.org/docs/handbook/module-resolution.html
In this case, moduleB is a single file... lets call it
When it does not find that, it does not continue to apply the resolution steps 5-7.... it should do this - which would mean additionally scanning for:
within Is this more clear now? |
the two processes happen at different times. the module resolution is a preprocessing step, and does not know about what ambient module declarations are there in the global scope. at the time the second step happens it is too late to change the results of the first step. hope that clarifies it. |
This doesn't make sense, as having an ambient declaration explicitly shimmed into the .d.ts file as with the example repo's workaround.d.ts package makes everything work fine.
When --outFile is applied, the entire declaration file is a system/amd representation of a package's physical file structure... tsc should expect the same naming conventions to apply. Keep in mind none of this is an issue if you don't use the --outFile argument, but right now there is no way to tell tsc to bundle emitted javascript but to not bundle emitted declaration files. The only way to consume a bundle produced by tsc with the --outFile argument is to shim into the package a folder structure that anticipates the consuming project's tsconfig --paths setup which seems ridiculous. |
this is correct. not sure why you need to bundle one and not the other. can you elaborate?
that is not accurate. it depends on your scenario. you can just have a file that |
In practice I don't think you would want to, but the fact that the declaration file requires the package to have this hacky folder structure, that additionally must be aware of the consumer's
I assume you are keeping in mind that --outFile only works with System and AMD module formats. That said, creating a file that exports the contents of the bundle will have the same issue... the declaration file is what appears to let TSC understand the shape of the System format bundle. TSC's expectation for the ambient modules declared within the dts file is not consistent with it's expectation for packages/barrells/etc that are defined within individual files. The bundled dts file will need to declare an ambient module explicitly named "myBundeledModule", but when tsc produces the dts it uses the filename as the ambient module name... which means it is most likely going to be "index." Furthermore, if "myBundeledModule.d.ts" exists somewhere that is defined by --paths argument, then the expectation for the ambient module name currently changes even more, and does so unpredictably. In the bundle, TSC does not honor the convention that the entry point for the bundle/package/barrel could be an ambient module named "index" On the file system, when a module resolves to a folder, TSC does honor that convention by checking the folder for an index.d.ts. What this leads to is a pattern where consuming bundles/packages created by tcs is inherently different than consuming "loose" packages/barrells/etc. |
@mhegazy I'm interested in confirming whether or not the issue I've described is understood. I'm attempting to split a project consisting of > 1000 ".ts" files into separate typescript projects, each emitting a single-file along with accompanying .d.ts definition. The definition file is critical in making sure the system format single file is consumable across sections of the application. -- The time it takes liteserver to deliver thousands of assets to the browser after a page refresh (during development) sucks... single file transpilation during development alleviate this. -- I'm using the solution proposed in the following comment to achieve The only issue I have right now is that transpiling > 1000 files every time I save a file takes 5-6 seconds, which negates the benefits of the single file served via liteserver. By breaking the project up into smaller packages I can reduce this to < 1s and still only require 10 or so assets to be fetched by the browser during development. The way TSC generates the d.ts file right now for an --outFile package means that consuming the package during development requires a different folder structure than during build. |
are these outputs consumed as commonjs modules? or amd modules? is there a bundler involved?
We have a change in #17269 that should make
The part i am missing is why not just generate the .d.ts in a single file instead of multiple files? |
During development, they are consumed as system modules. There is no bundler involved... I use
When the entire source is included in a monolithic ts project (single tsconfig) the time for
All of this works exactly I would expect, EXCEPT that none of the declaration files in either scenario currently produce an appropriate ambient module declaration for their entry points -
Consider either of these import statements...
TSC successfully locates the declaration file, but does not resolve If I manually go into the either of the declaration files, and rename the ambient module declaration to The only way to coerce it to work is to use the hacky workaround I outlined in my original repro. If I had not used --outFile argument, the resolution would be fine, because when TSC encounters a folder it checks for an This is an oversight/bug in the resolution steps as it relates to declaration files generated alongside the --outFile argument. The path of the d.ts file is expected to be included in the ambient module name, and that is an unreasonable expectation (the d.ts file can be moved, can be mapped virtually with the --paths argument, etc). I'm of the impression that I still haven't clarified for you what the issue is, but I'm not sure how to be more clear. This is not a workflow question, and I think the issue has been done a disservice to have been so hastily labeled as a "question." If the decision is "will-no-fix", then I guess that is that, but there is an issue I'm reporting, not a question. |
Yes. and there is a reason why |
@josh-sachs have you considered emitting declaration files in a separate One pass could create the bundle and another pass or even a parallel instance could create the declaration files. Just a thought. |
@mhegazy I feel like I'm getting punked here... I know that @aluanhaddad I'm not trying to create individual declaration files alongside a single .js file. I'm trying to consume the single definition file generated by TSC via --outFile but TSC is not correctly resolving the ambient declarations. Can someone confirm that they have actually forked the repro I provided locally and observed the phenomenon I'm describing? |
As I have described earlier. The scenario you describe is not supported.
I agree this does not seem like a question. relabeling as such. |
The issue I'm reporting is exactly that, one per your own documentation, and the actual observed behavior of tsc,
It's become seeming clear that you've not taken the time to review what I've put together as part of a working environment. Its frustrating because the error here is so extremely simple to observe if you would just do so. When you import a library defined via ambient modules in a .d.ts file, type resolution should continue to work as if you were importing the non-ambient modules -- after all, that is the entire freaking point of the ambient declarations. Right now, you are basically asserting that for a situation where a single *** I REPEAT AGAIN EVERY SINGLE ASPECT OF THIS WORKS WITH ONE ASININE EXCEPTION *** The exception is that tsc generates the .d.ts file with an ambient module named "index", but when consuming the d.ts file does not interpret the ambient module named "index" as the library's entry point during type resolution. All I've asked - repeatedly - was that you give some indication that I've successfully communicated to you what the issue is that I'm reporting. It appears there has been some failure in communication between us and instead of making honest efforts to rectify, you've chosen to hastily take innacurate administrative action against the ticket, denying anyone else on your team the opportunity to review the issue and see if they might not be able to come to a better understanding of what I'm reporting. From: https://www.typescriptlang.org/docs/handbook/module-resolution.html
When this process succeeds at steps 3 or 7, any type resolution should evaluate the d.ts file for...
... because when tsc generates the d.ts for a library via -- This is a BUG. |
Calm down dude... Code of Conduct |
Honestly I've been reading this ticket as well but right now it's just a wall of angry text and it doesn't seem like we're going to reach mutual understanding with this attitude. If this is really as straightforward as you're trying to make it sound, there should be a three- or four-file repro. |
I apologize if too much of my frustration trying to communicate the issue has come through in my responses. Honestly, I've been trying to go to great lengths to make sure that my followups remain courteous and focus strictly on shoring up any points where there may be lack of clarity - I'm not sure certain which point in the code of conduct I could possibly have violated. I haven't insulted anyone, used vulgar language, or even yelled (all caps, etc) for that matter. There is one line in my last post that was caps, but only used as emphasis. If I've violated some grammar, language or forum style rule that has resulted in something being misconstrued otherwise I'm really sorry.
Again - I'm sorry if somehow I've triggered something that has made anyone feel the need to be defensive. I've reviewed the entire thread and while my responses are verbose they only contain information to help achieve clarity. I'm at a loss for how it could be construed as a wall of angry text - but again I realize communication is a two-way endeavor, so if what I'm saying is being received in an unintended way than I recognize I must not be saying it correctly. All of that said I'm glad to know there are additional eyes on the issue. Assuming we can move past any misunderstandings as it relates to decorum, I want to provide a brief primer to make sure that all of the terminology I'm using is up-to-snuff.
I've provided a repo at the beginning of the thread, it consists of more than three or four files, but only because that is what any project seems to require these days simply in order to make it build. https://github.com/josh-sachs/typescript-18311 The
The "workaround" barrel contains one additional folder, inside which there is one additional file. The intent of which is to help illustrate the problem.
If you fork the repo, and transpile the two barrels, it will create two corresponding libraries and two corresponding declaration files (4 files total).
the declaration file
the declaration file
Once the libraries have been transpiled, open up the file Observe the type-checker does not successfully resolve that there is a module named Observe the type-checker does successfully resolve that there is a module named It is clear by this example that the latter works because the declaration file contains an additional ambient module declared by the name The issue is that '@bundles' is a In practice, the convention is that a barrel contain an |
Thank you for taking the time to reassess! If there is any way for me to buy you guys a beer or twelve, I would be happy to do so. Again - I apologize for any perceived hostility in the thread, that was not my intent. |
#15951 is related, I'm still seeing the behavior (of erroneous file paths when using "system" and side-by-side definition files). it's hard to understand why, although I'm using a symlink instead of subfolders |
@pocesar I reviewed #15951 but I'm not sure they are related beyond having impact on --outFile and .d.ts generation. The specific issue I'm trying to highlight relates to ambient module resolution within a declaration file produced by tsc. I believe resolution candidate for this ticket would be a change so that the tsc type-checker recognizes an ambient "index" module declaration as an entry point for resolving additional ambient modules within the declaration file. Only providing clarification to prevent them from being combined as duplicates. |
@josh-sachs yeah, just pointed out that it's the same issue when using |
There are a number of factors that are causing this issue:
This is further complicated by the fact that in TS 4.1 we are adding a new I don't think we can change any of those behaviors without a number of unintended consequences. We may instead need a way to opt-in to a differing behavior. For something like this to work we would need a way to specify that the bundled modules should be treated as relative paths in some way. One option might be to have a syntactic marker to indicate relative module resolution within the file: // broken.d.ts
declare module "broken/resource-1" {
export class Resource1 {
}
}
declare module "broken" {
export * from "broken/resource-1";
}
// indicate this file is a bundle and paths are resolved relative to the bundle:
declare module ".";
// or (to be more specific about the entry point):
declare module "broken" as "."; Or, alternatively: // broken.d.ts
declare module "./resource-1" { // resolved as `broken/resource-1` (from the file name)
export class Resource1 {
}
}
declare module "." { // resolved as `broken` (from the file name)
export * from "./resource-1";
} Another might be to modify {
"compilerOptions": {
"paths": {
// one placeholder with *-1 cardinality could indicate a bundle?
"@bundles/broken/*": ["../bundles/broken"],
// two placeholders with 1-1 and *-1 cardinalities (respectively) could indicate nested bundles?
"@bundles/*/*": ["../bundles/*"],
// object literal to specify behavior?
"@bundles/*": [{ "path": "../bundles/*", bundle: true }]
}
}
} /cc @weswigham, @andrewbranch for some additional perspectives. |
Long thread but I think @rbuckton 's comment summarizes it well. We'd be open to a concise suggestion for some new declaration style that enables this scenario to work more ergonomically, but the behavior as-is in this scenario is the intended behavior, not a bug. The longstanding lack of feedback on this one would indicate that this isn't a mainline scenario blocker. |
@RyanCavanaugh What is a user supposed to do if they come across a package that points at a |
TypeScript Version: 2.4.0 / nightly (2.5.0-dev.201xxxxx)
2.5.2
Code
Expected behavior:
TSC to correctly resolve the import statements within
app/main.ts
against the configured--paths
, applying the same logical resolution strategy that lets it resolve barrels.(note: I've had to escape the [@] symbols for some reason at the start of these lines).
Actual behavior:
It appears the path part of the import statement is currently expected to be included in the declared module name.
Even if I name
index.ts
tocommon.ts
for example, this will still fail because the resolution logic is expecting the"@bundles"
path token to be in the declared module name.I can coerce the expected behavior by creating a "@bundles" folder inside my module - but the whole point is that the "--paths" option shouldn't be married to the module structure.
The text was updated successfully, but these errors were encountered: