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

SystemJS/CommonJS interop #8687

Closed
elsasslegend opened this issue May 19, 2016 · 19 comments
Closed

SystemJS/CommonJS interop #8687

elsasslegend opened this issue May 19, 2016 · 19 comments
Labels
Duplicate An existing issue was already created Effort: Difficult Good luck. Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@elsasslegend
Copy link

Hello, I have an issue involving SystemJS/CommonJS interop and definition files. I can't find any acceptable solution for now.
My app consists of

  • a server part that compiles to CommonJS
  • a client part that compiles to SystemJS (flag allowSyntheticDefaultImports on)
  • a shared part that is imported (copied) into both client and server parts.
    I cannot figure out how to import some libs like bluebird or lodash without having either the compiler complaining about something or an exception at execution level. I used definitions files provided by DefinitivelyTyped for bluebird/lodash.

I tried all import syntaxes I could find, but without success

import Promise from 'bluebird'; do not compile on CommonJS,

Module '"bluebird"' has no exported member '_'.

import {Promise} from 'bluebird'; still do not compile on CommonJS,

Module '"bluebird"' has no exported member '_'.

import * as Promise from 'bluebird'; compile on both CommonJS and SystemJS, but crashes during execution, on some functions that directly use Promise as constructor

import Promise = require ('bluebird') old syntax compiles on both CommonJS and SystemJS. But crashes at execution on SystemJS, because Promise cannot be used directly as constructor/function.

Error: TypeError: Promise is not a constructor(…)

Do you have any solutions for this problem ? What is the right way to do in this case ?

@unional
Copy link
Contributor

unional commented May 20, 2016

I think you are one of the first who experience the issue I described (in a different scenario):
#7398

@elsasslegend
Copy link
Author

The only thing that works for me is modifying .d.ts files and replacing export=Promise by var Promise :
declare module 'bluebird' { var Promise: PromiseConstructor; }
But I don't think it's a very good solution having to modify definition files..

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Effort: Difficult Good luck. labels Jun 7, 2016
@DanielRosenwasser
Copy link
Member

Right now we need to get a better understanding of what the interop behavior will be. This is contingent on what the NodeJS Core Technical Committee (CTC) decides on.

@mhegazy mhegazy added Suggestion An idea for TypeScript and removed Bug A bug in TypeScript labels Jun 7, 2016
@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jun 10, 2016

@DanielRosenwasser yeah, the addition of allowSyntheticDefaultImports made a lot of sense because SystemJS is supporting the semantics, and the flag allows TypeScript to model this behavior.

Edit: Hit enter prematurely.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jun 10, 2016

@DanielRosenwasser sorry I hit enter prematurely: what I meant to say was that I don't see how TypeScript can solve this problem without runtime semantics provided by node to accommodate easy consumption of default exports. If the semantics are introduced then TypeScript can model them.

@unional
Copy link
Contributor

unional commented Jun 10, 2016

@DanielRosenwasser Should (CTC) decides on this or should it be ECMAScript spec group (or if they are closely related / one and the same. 😄)?

We need to have a full spec of CommonJS -> ES2015 and ES2015 -> CommonJS interop.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jun 10, 2016

I don't necessarily know if the interop problem is one that TC39 itself needs to specify explicitly, though I think there are decisions that could be made there so that importing a CommonJS module from an ES module, and an ES module from another ES module, are more balanced experiences.

Unfortunately, that doesn't seem to be the case according to the interop draft right now.

@unional
Copy link
Contributor

unional commented Jun 10, 2016

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Jun 10, 2016

From the wording of the proposal, or at least the disclaimer, it seems that the node community is still thinking about whether they will even adopt support for ES6 modules at all. That saddens me quite a bit. It fragments the community.

@unional
Copy link
Contributor

unional commented Jun 10, 2016

It is not just node, right? Browsers also. So it really should be part of language spec.

@aluanhaddad
Copy link
Contributor

@unional well the browsers have committed to implementing the standard I suppose at some point. Certainly the module syntax, as it is part of ES2015. But since the browsers themselves don't currently have a competing implementation, projects like SystemJS polyfill it in a forwards compatible way, in the meantime providing de facto standard modules in the browser. The problem is that node has a competing implementation of modules. Considering how much time and energy TC39 spent trying to specify modules in a manner compatible with existing module formats, including CommonJS, it would be a real shame if node never adopted it.

@unional
Copy link
Contributor

unional commented Jun 10, 2016

Ah, you are right. Browsers doesn't support commonjs natively..... Dumb me.... 😁

@unional
Copy link
Contributor

unional commented Feb 17, 2017

NodeJS have casted their vote: https://github.com/nodejs/CTC/pull/60/files#diff-2b572743d67d8a47685ae4bcb9bec651R217

Most important note:

Therefore cannot import named variables from CommonJS (CJS) modules, e.g.
import {readfile} from “fs”.

So CommonJS modules will provide only a namespace export with default and all
properties will be accessed through that. For example:

import * as express from “express”
var app = express.default()

import express from “express”
var app = express()

Breaking change to TS (and all existing TS code!) in order to align.

UPDATE: [email protected] aligns with this direction.

@unional
Copy link
Contributor

unional commented Feb 27, 2017

This issue really should be a P1 and fixed asap.

We have people start compiling TS to es6 and any reference to CommonJS will break.
This is now not just systemjs commonjs interop.

It is Babel/NodeJS/SystemJS vs TypeScript interop on CommonJS.

http://stackoverflow.com/questions/42477112/uncaught-typeerror-i18next-init-is-not-a-function/42497453#42497453

@DanielRosenwasser @aluanhaddad @mhegazy

@unional
Copy link
Contributor

unional commented Apr 3, 2017

I have creates a repro for this issue: https://github.com/unional/domture/tree/interop-issue
The situation is slightly different than the OP.

In the repro, it demonstrates the difference between:

// inside `@unional/logging`
export * from 'aurelia-logging'
import { getLogger as getLog } from 'aurelia-logging'
export { getLog }

When you run the test, you will see that getLog is undefined while getLogger is not.

The situation will be the same if you import using import...require syntax. i.e.:

import L = require('aurelia-logging')
console.log(L.getLogger)  // [function]

import { getLogger } from 'aurelia-logging'
console.log(getLogger) // undefined

@unional
Copy link
Contributor

unional commented Apr 3, 2017

cc @blakeembrey
Just notice you are not in this thread 🌷

@aluanhaddad
Copy link
Contributor

@unional I've been thinking about this quite a lot...

I think there's a disconnect here that must be directly addressed.

I think there has been a serious problem created by transpiling ES Module syntax to CommonJS semantics.

What this comes down to, to some extent, is that many think that they're using ES Modules when they are not.

There are many different factors in play here... but I think this problem is simply not going to go away and that it is absolutely critical that something is done to address it. I don't know what that should be exactly...

@mhegazy
Copy link
Contributor

mhegazy commented Dec 2, 2017

Should be covered by #19675

@mhegazy
Copy link
Contributor

mhegazy commented Dec 2, 2017

Closing in favor of #16093

@mhegazy mhegazy closed this as completed Dec 2, 2017
@mhegazy mhegazy added the Duplicate An existing issue was already created label Dec 2, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created Effort: Difficult Good luck. Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants