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

BUG/SUGGESTION: Missing support for destructuring in import … = <namespace> statements #20010

Closed
SMotaal opened this issue Nov 14, 2017 · 12 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@SMotaal
Copy link

SMotaal commented Nov 14, 2017

TypeScript Version: 2.6.1

Intent

To be able to export imports (using allowSyntheticDefaultImports) and not run into the dreaded voldemort cannot be named when importing those exports and consuming them in other modules inside or outside the scope of the project.

Code

Currently, the most intuitive approach exposes the entity but leaves out the type information, resulting in the cannot be named:

import fs from 'fs';
export const { existsSync, readFileSync, writeFileSync } = fs;

The TypeScript way of going about this disconnect is to use the export import readFileSync = fs; and do this for each and every member that needs to be exposed both as entity and type:

import fs from 'fs';
export import existsSync = fs.existsSync;
export import readSync = fs.readSync;
export import writeFileSync = fs.writeFileSync;

Which is crude when compared to the simplicity of export const {…} = fs; to say the least.

So, here is where it is not clear if this is intentional, a bug, or a future TODO from the past, but as a TypeScript developer one would expect this to be a given valid and supported syntax:

import fs from 'fs';
export import { existsSync, readFileSync, writeFileSync } = fs;

Expected behavior:

The export const … is behaving (though not ideal) but on par with TypeScript's conventions since it explicitly exports the entity.

The export import <name> = <namespace>.<name> gladly resolves this disconnect.

But one would expect export import {… <names> } = <namespace> to be valid TypeScript syntax, which behaves like export const {… <names> } = <namespace> but also expose the types.

Actual behavior:

Sadly, while TypeScript can properly handle destructuring in consts, and even transpile those flawlessly to legacy individual var statements, it does not like destructuring in the import counterparts.

edit: simplified code for readability

@SMotaal
Copy link
Author

SMotaal commented Nov 14, 2017

@SMotaal
Copy link
Author

SMotaal commented Nov 14, 2017

Some issues to keep in mind:

  • should import = compile into const = (instead of current var =)
  • should export import = produce the definition export import = (instead of current)
  • any possible regressions from introducing destructuring in import =

@mhegazy
Copy link
Contributor

mhegazy commented Nov 14, 2017

Duplicate of #18307

@mhegazy mhegazy marked this as a duplicate of #18307 Nov 14, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Nov 14, 2017

Currently, the most intuitive approach exposes the entity but leaves out the type information, resulting in the cannot be named:

not sure i understand the issue here..

import fs from 'fs';
export const { existsSync, readFileSync, writeFileSync } = fs;

generates the following declaration file:

/// <reference types="node" />
import fs from 'fs';
export declare const existsSync: typeof fs.existsSync, readFileSync: typeof fs.readFileSync, writeFileSync: typeof fs.writeFileSync;

@mhegazy
Copy link
Contributor

mhegazy commented Nov 14, 2017

To be able to export imports (using allowSyntheticDefaultImports) and not run into the dreaded voldemort cannot be named when importing those exports and consuming them in other modules inside or outside the scope of the project.

can you elaborate? seems like this is what we need to fix instead of building a new behavior to work around it.. are you looking for #9944?

@mhegazy
Copy link
Contributor

mhegazy commented Nov 14, 2017

As i noted in #18307 (comment), we would rather not invest into non-standard syntax at this point, and instead help users move to the ES6 module syntax all the way. For instance #19675. if there is something that is blocking using the ES6 module syntax we should fix that instead.

@mhegazy mhegazy added the Needs More Info The issue still hasn't been fully clarified label Nov 14, 2017
@SMotaal
Copy link
Author

SMotaal commented Nov 14, 2017

Okay… So I went over your refs and here is an ES6-centric rehash:

My intended compiled output (.mjs compliant) is:

import path from 'path';
export const { resolve } = path;

So with { target: "ESNEXT" module: "ES2015" } my src/helpers.ts looks like this:

import path from 'path';
export const { resolve } = path;

And in another file I also do this src/package.ts:

import {resolve} from './helpers.ts';

export class Package {
  resolve = resolve;
}

Now TSC throws for that second file resolve … could not be named..

Of course one can argue that the second file is the culprit, and that the obvious fix is to change the line to resolve: typeof resolve = resolve. Absolutely, this is one way to go about it (meh).

Then, after testing a million things it turns out that if instead of export const {resolve …} = path in the helpers.ts I did an export import resolve = path.resolve then this line resolve = resolve in src/package.ts does not end up throwing.

So it seems that when using import x from 'x' the way TypeScript propagates types when using export const {…} = x is inferior to when using export import y = x.y, since the latter does not throw about y … could not be named.

In ES6 spirit, I would recommend isolating what causes the differences in behaviour and making it so that export const {…} = x propagates types in the same way as if it was dealing with the more explicit (but far-from-standard syntax) counterpart export import y = x.y. This would eliminate the need for import = altogether and make this issue mute, because yes, everything can finally be named :)

This is all just about the propagation of types when rexporting, it has not implications on the resulting code, and should result in far less issues relating to things that cannot be named.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 14, 2017

Now TSC throws for that second file resolve … could not be named..

I think this is #9944. and we have talked about this in the past, and decided to change the behavior. i.e. add an import to "fs" in the declaration file for src/package.ts, even if you do not have one in your source.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 14, 2017

I think this seems to all stem from #9944. i think fixing it is more ergonomic to users, and avoids adding new syntax to an already syntactically crowded area of the language.

@SMotaal
Copy link
Author

SMotaal commented Nov 14, 2017

That would solve it… The one thing I would be curious about though is if fixing it would be more appropriate in the exporting file instead of the consumers. I'm just thinking out loud, but since export import y = x.y; seems to fix it once and for all regardless of how many subsequent consumers files like the src/package.ts, the potential for problems would shrink exponentially.

So if TypeScript would mimic whatever happens (again only talking type propagation) in that one file when using export import instead of export const then you would not even need to make any changes to any consumer files (and potentially even the exporting file itself) because TypeScript simply propagates those names.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 14, 2017

I would be curious about though is if fixing it would be more appropriate in the exporting file instead of the consumers.

You do not have control over the exporting file all the time, it might be just a .d.ts. moreover, changing the exporting file only is not sufficient. you need to change both.

@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

3 participants