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

Proposal: import {...} = require('name') destructuring sugar #18307

Closed
guybedford opened this issue Sep 7, 2017 · 6 comments
Closed

Proposal: import {...} = require('name') destructuring sugar #18307

guybedford opened this issue Sep 7, 2017 · 6 comments
Labels
Suggestion An idea for TypeScript

Comments

@guybedford
Copy link
Contributor

guybedford commented Sep 7, 2017

I'd like to propose the following syntax for importing named exports from CommonJS modules in TypeScript:

import { readFile } = require('fs');

When transpiling to CommonJS the above would become:

const { readFile } = require('fs');

And when transpiling to ES modules it would become:

import _fs from 'fs';
const { readFile } = _fs;

Thinking purely in terms of what this means in an ES modules world, the import x = require('x') is identical to import x from 'x' while import { x } = require('x') is a destructuring of the default export in the ES module semantics.

Such a syntax would offer a replacement for the import { fs } from 'fs' which will not be ever supported in NodeJS since a CommonJS module is represented by the namespace object ModuleNamespace { default: fs }. So such a syntax makes it clear that this is a TypeScript-specific convenience during the transition period (many years yet) into ES modules.

Having this alternative in place would allow TypeScript to then have the same module semantics between outputting ES modules and CommonJS modules on this working subset, without losing the feature expressivity of named exports for CommonJS modules.

Related: #17556

@mhegazy
Copy link
Contributor

mhegazy commented Sep 9, 2017

We would like to avoid adding any new non-standard syntax.

The reason we added import .. = require originally was that we needed a module syntax, and at the time, TC39 did not reach a concences on what the module would be; if memory serves, at that time, it was not even clear if modules would make it to ES6/ES2015 spec.

If we were to do the whole thing 1 year later, we would not have included the import ... = require(..) syntax (nor namespaces/module for that matter).

That said, I think the right approach here is to address #16093, and make import d from "mod" mean either default export for an ES6 module (i.e. has an "__esModule" property) or the whole module for CJS/AMD modules. this way you can just say import fs from "fs"; thus eliminating the need for a new syntax.

We have wanted to do this change for a while now, but wanted to get some clarity on the plans for node implementation to ensure that whatever breaks we introduce are a. warranted, and b. final.

@guybedford
Copy link
Contributor Author

Thanks @mhegazy for the update here.

Supporting the __esModule interop will align TypeScript with Webpack and Babel when outputting CommonJS modules which will be nice to see.

Note though that this issue is primarily concerned with the output of ES modules from TypeScript, which I think is a separate concern.

When using NodeJS modules, the inability to have a named exports form from CommonJS is the problem I'm concerned with, especially since the TypeScript ecosystem is already quite invested in supporting these typings.

This proposal is thus a way to deal with that problem, providing an alternative solution and upgrade path, that itself is destined to become a deprecation path since we know import = require is going to be deprecated anyway.

I understand the unwillingness to jump before being hit with the exact issue though.

@DanielRosenwasser DanielRosenwasser added the Suggestion An idea for TypeScript label Sep 9, 2017
@guybedford
Copy link
Contributor Author

I've been thinking more about this and I think the initial path should rather be to have allowSyntheticDefaultImports as enabled by default (or a better way that understands this is the CommonJS module boundary only). If that was the case import fs from 'fs' in typescript would be the happy path by default, encouraging the right type of code over import * as fs from 'fs'.

This proposal was to try and find a way that can be backwards compatible, without having to worry about the above, but thinking about it further it will likely just muddy the waters even more. It's no different to traditional CommonJS (before destructuring) and if we can start to get everyone on board with this as soon as possible, the NodeJS transition will be easier.

Is there an issue somewhere tracking this default export handling for CommonJS typings by default?

@guybedford
Copy link
Contributor Author

Closing for reasons stated above.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Sep 9, 2017

@guybedford part of the issue is that there is no such thing as a CommonJS typing per se, and I'm not sure such a construct should exist.

Declaration files need to work for all module formats as well as for conditional globals and hence the term for these declarations is UMD.
A UMD declaration is identified a combination of two distinct syntactic forms.

// the single declaration.
declare function lib(...args: object[]): void;

// allow the exported declaration to be imported as a module
// from code written as modules 
export = lib;

// allow the exported declaration to be referenced as a property of the global object via
// the name on the right hand side of `as`
export as namespace lib;

The export = above does not imply CommonJS. It works fine from system modules or when --allowSyntheticDefaultImports is enabled and can then be imported as the default. unfortunately, it also allows erroneous uses as well as correct uses.

import fs from 'fs'; // correct, valid TypeScript 

import {readFile} from 'fs'; // incorrect, valid TypeScript

import fs from 'fs';
const {readFile} = fs; // correct, valid TypeScript

I do not think the declarations need to change necessarily but TypeScript needs a way to become stricter.

EDIT: fixed typo in import

@guybedford
Copy link
Contributor Author

Yeah I've just been playing with some of the edge cases of it myself, and remember why I turned it off now in my own workflows too! It does sound like a new direction is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants