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

modules not exported the same way between es5 and es6 builds #903

Closed
rikkertkoppes opened this issue Mar 3, 2017 · 26 comments
Closed

modules not exported the same way between es5 and es6 builds #903

rikkertkoppes opened this issue Mar 3, 2017 · 26 comments

Comments

@rikkertkoppes
Copy link

Working with RC.8 here, I am using the following stack:

typescript 2.1.4
webpack 2.2.1

When packing inferno, I run into issues with how the modules are exported. When working in typescript, the typings from dist-es/index.d.ts are correctly resolved, indicating that, for example, Component is the default export from inferno-component. Typescript build phase works ok.

Next inline is the webpack build phase, which picks up the main field from package.json, pointing to dist/inferno-component.node.js. Which is ok, as I don't want es6 code in browsers just yet.

However, the es5 build exports Component as the root export, which is not how it is expected to be.

This all worked fine with inferno 1.2.2, so this breaks the upgrade path, which I think is not desired

I would suggest aligning the module signature across es5 and es6 builds, unless I am missing something here

@rikkertkoppes rikkertkoppes changed the title modules not exported correctly modules not exported the same way between es5 and es6 builds Mar 3, 2017
@Havunen
Copy link
Member

Havunen commented Mar 3, 2017

HI @rikkertkoppes module root was dropped away from package.json. Are you using jsnext:main that is non-standard and used for building inferno internally?

@Havunen
Copy link
Member

Havunen commented Mar 3, 2017

@longlho Any idea what could cause this?

@rikkertkoppes
Copy link
Author

No, not using jsnext:main, it's the opposite. Webpack picks out the main field and produces es5 code which is ok, but does not follow the module signature that is expected by typescript (expecting Component as default export, not root export)

When I modified inferno-components package.json and added a module field pointing to the same es6 build (dist-es/index.js), everything gets packed just right. However, that leaves me with es6 in the browser, which is not (yet) desired.

@longlho
Copy link
Member

longlho commented Mar 3, 2017

@rikkertkoppes do you have your tsconfig.json & webpack.config.js?

@rikkertkoppes
Copy link
Author

Both are handled by grunt, but these are the options set for them

ts:

module: 'commonjs',
target: 'es5',
sourceMap: true,
comments: true

webpack:

entry: "./public/js/dist/main.js",
devtool: "source-map",
output: {
    path: "public/js/",
    filename: "packed.js"
},
//use ts sourcemaps
module: {
    preLoaders: [{
        test: /\.js$/,
        loader: "source-map-loader"
    }, {
        test: /\.json$/,
        loader: "json-loader"
    }]
}

@longlho
Copy link
Member

longlho commented Mar 3, 2017

do you mind making a small example repo to demo the issues? Looking at the configs things seem fine

@rikkertkoppes
Copy link
Author

Yeah sure, probably after the weekend though

@rikkertkoppes
Copy link
Author

rikkertkoppes commented Mar 10, 2017

here you go: https://github.com/rikkertkoppes/infernoBuildProblem

I included the ts build and the webpack bundle files in the repo. Description in the readme

Just checkout and open the html file, note that the console logs undefined and an error, which is exactly what I observe

@longlho
Copy link
Member

longlho commented Mar 10, 2017

awesome thanks!

@longlho
Copy link
Member

longlho commented Mar 10, 2017

yeah confirm the prob is that we dist UMD ES5 but type def still assumes it's ES6. The solution is prob to set main as non-rollup (transpiled only) and set browser to the minified file.

@Havunen thoughts?

@Havunen
Copy link
Member

Havunen commented Mar 11, 2017

@longlho Sounds good to me

@katyo
Copy link

katyo commented Mar 15, 2017

I also have an issue with importing default export in typescript.
Is there any workaround to fix that now?
Maybe it would be better avoid default export at all or simply add named export.

Currently default key is missing in the exported module object. I tried to import whole module like this:

import * as h from 'inferno-hyperscript'

In this case I have the function, but typings is missing.
In another case when I import it as usual I have the typings but reference to function is missing (i.e. undefined), because the default key is missing.

I found temporary solution using proxy module, like that:

import {VNode, InfernoChildren} from 'inferno';
import * as h_ from 'inferno-hyperscript';

export interface H {
  (_tag: string | VNode | Function, _props?: any, _children?: InfernoChildren): VNode;
}

export const h: H = h_ as any;

@longlho
Copy link
Member

longlho commented Mar 19, 2017

Can you guys try canary tag w/ npm i inferno@canary to see if the issue's fixed?

@moatra
Copy link

moatra commented Mar 22, 2017

As of inferno@1.4.2, the main entry point appears to be working correctly. However, for webpack builds for clientside bundling now pick up on the browser entry point. That field for inferno-component points to dist/inferno-component.min.js, which returns the Component UMD but does not have the default property set on the module.

As a temp workaround, you can update resolve.mainFields in your webpack config to ignore the browser entry point (ala mainFields: ["module", "main"]), and it imports the non-minified es5 target that correctly sets the default field.

@Havunen
Copy link
Member

Havunen commented Mar 22, 2017

yeah I think this is still wrong. When using webpack we should import dist/index.js the non minified or bundled es5 code, and webpack will handle the requires. Then those full bundles (js/min.js) we should send into CDNjs

Anyway it "works" now, so I think this fix is not critical and can wait for 1.5

@TrySound
Copy link

@moatra Does it work with webpack-dev-server? I got 'fs' and 'net' are not resolved

@TrySound
Copy link

@Havunen I guess it's critical because of webpack-dev-server :)

@Havunen
Copy link
Member

Havunen commented Mar 22, 2017

@TrySound strange. We are using Webpack Dev server with Inferno also at work, but didn't notice any issue

@TrySound
Copy link

This without browser field

./~/debug/src/node.js
Module not found: Error: Can't resolve 'fs' in '...\node_modules\debug\src'
 @ ./~/debug/src/node.js 184:15-28
 @ ./~/debug/src/index.js
 @ ./~/sockjs-client/lib/main.js
 @ ./~/sockjs-client/lib/entry.js
 @ (webpack)-dev-server/client/socket.js
 @ (webpack)-dev-server/client?http://localhost:8080
 @ multi (webpack)-dev-server/client?http://localhost:8080 ./src/example.js
errors @ client?cd17:119
sock.onmessage @ socket.js:37
EventTarget.dispatchEvent @ eventtarget.js:51
(anonymous) @ main.js:274
SockJS._transportMessage @ main.js:272
EventEmitter.emit @ emitter.js:50
WebSocketTransport.ws.onmessage @ websocket.js:35
client?cd17:119 ./~/debug/src/node.js
Module not found: Error: Can't resolve 'net' in '...\node_modules\debug\src'
 @ ./~/debug/src/node.js 191:16-30
 @ ./~/debug/src/index.js
 @ ./~/sockjs-client/lib/main.js
 @ ./~/sockjs-client/lib/entry.js
 @ (webpack)-dev-server/client/socket.js
 @ (webpack)-dev-server/client?http://localhost:8080
 @ multi (webpack)-dev-server/client?http://localhost:8080 ./src/example.js
errors @ client?cd17:119
sock.onmessage @ socket.js:37
EventTarget.dispatchEvent @ eventtarget.js:51
(anonymous) @ main.js:274
SockJS._transportMessage @ main.js:272
EventEmitter.emit @ emitter.js:50
WebSocketTransport.ws.onmessage @ websocket.js:35

@longlho
Copy link
Member

longlho commented Mar 23, 2017

that's bc of debug and modules that export non-browser-compatible entry point in main

@Havunen
Copy link
Member

Havunen commented Mar 23, 2017

we have one more option, add webpack entry point ... :-)

@Havunen
Copy link
Member

Havunen commented Mar 26, 2017

somebody add test cases for this

@Havunen
Copy link
Member

Havunen commented Mar 26, 2017

can you guys try out if latest canary works for you npm i inferno@canary

@thdxr
Copy link

thdxr commented Mar 26, 2017

The latest canary has fixed the issue for me 👍

@moatra
Copy link

moatra commented Mar 27, 2017

Installed inferno@1.5.0-alpha.2f6639f8 and friends:

Observed the following for var ic = require('inferno-component');:

  • ic is an es5-friendly function for webpack builds with target='node'✔️
  • ic is an es5-friendly function for webpack builds with target='web'✔️
  • ic.default is defined and set to the same value as ic for webpack builds with target='node'✔️
  • ic.default is defined and set to the same value as ic for webpack builds with target='web'✔️

@Havunen
Copy link
Member

Havunen commented Mar 29, 2017

I think this is now fixed. Closing. Please comment or file new issue if you guys still have problems with this.

@Havunen Havunen closed this as completed Mar 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

7 participants