add an es-modules build (referenced with pkg.module) #1027

rossipedia · 2017-05-31T13:33:19Z

This output file (mobx.module.js) is built with the ES2015 typescript target, so no ES2015 features get transpiled, and es module exports are preserved. This allows consumers of mobx to take advantage of
tree-shaking bundlers such as rollupjs and webpack 2.

I was considering using the .mjs extension, as that looks like it will be the most likely extension for es modules, but as it's not supported in node at all at the moment, I decided against it.

Point of discussion:

Should es2015+ features still be transpiled?

I'm not sure what the standard practice here is. Webpack usage typically avoids processing code from node_modules, so a consumer trying to include the mobx module version would have to be made aware that they need to include mobx in their transpilation process if they're targeting ES5. However, from what I've seen rollup configs usually include the node-resolve plugin before things like babel / typescript, so the module code would generally be included in that transpilation process as well.

this output file (mobx.module.js) is built with the ES2015 typescript target, so no ES2015 features get transpiled, and es module exports are preserved. this allows consumers of mobx to take advantage of tree-shaking bundlers such as rollupjs and webpack 2. I was considering using the `.mjs` extension, as that looks like it will be the most likely extension for es modules, but as it's not supported in node at all at the moment, I decided against it

capaj · 2017-05-31T14:22:20Z

Should es2015+ features still be transpiled?

module entry point should point to a ES2015 bundle. That's the standard practise if you look at library like redux for example.
Any newer language feature must be transpiled away into idiomatic ES2015.

- this isn't actually necessary, it's an artifact from when I was putting the build script together in .ts before converting it back to .js

coveralls · 2017-05-31T17:56:37Z

Coverage remained the same at 95.769% when pulling 4537301 on rossipedia:module-build into 021cdbb on mobxjs:master.

capaj · 2017-06-02T14:31:15Z

@rossipedia I tried it in node.js with babel and it failed with this:

capaj@capaj-i7-4771:~/git_projects/mobx$ babel-node test-mod.js 
/home/capaj/git_projects/mobx/lib/mobx.module.js:195
StubArray.prototype = [];
                    ^

TypeError: Cannot assign to read only property 'prototype' of function 'class StubArray {}'
    at Object.<anonymous> (/home/capaj/git_projects/mobx/lib/mobx.module.js:201:1)
    at Module._compile (module.js:569:30)
    at loader (/home/capaj/.nvm/versions/node/v8.0.0/lib/node_modules/babel-cli/node_modules/babel-register/lib/node.js:144:5)
    at Object.require.extensions.(anonymous function) [as .js] (/home/capaj/.nvm/versions/node/v8.0.0/lib/node_modules/babel-cli/node_modules/babel-register/lib/node.js:154:7)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/home/capaj/git_projects/mobx/test-mod.js:1:1)

This looks like a babel bug because doing

class StubArray {
}
StubArray.prototype = [];

works in a browser/node.js. Only babel fails on this. We'll need to resolve this before merging because most people are transpiling with Babel and all of them could get their builds broken IMHO.

capaj · 2017-06-02T15:01:26Z

@rossipedia hmm assiging to a class prototype is actually illegal per ES6 spec in strict mode. I guess we'll have to fix it here.
Couldn't we just extend Array there? ES6 specifies array as subclassable...

…ct.getOwnPropertyNames instead

rossipedia · 2017-06-02T16:36:39Z

what babel configuration are you using for this test? I couldn't repro using babel-node lib/mobx.module.js --presets=ES2015, which as far as I understand should execute that line of code and trigger the error?

- babel doesn't like it when we set .prototype - es2015 spec says Array is extendable

rossipedia · 2017-06-02T16:55:11Z

Ahh ok, that's what the StubArray.prototype = [] hack was for. Let's see if there's a more elegant way around it

This reverts commit 99fd2fa.

- doesn't assign to StubArray.prototype anymore - polyfill from MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/setPrototypeOf

coveralls · 2017-06-02T17:15:30Z

Coverage decreased (-0.03%) to 95.735% when pulling d313d1c on rossipedia:module-build into 021cdbb on mobxjs:master.

capaj · 2017-06-02T17:20:35Z

I was using babel-preset-env. I think es2015 doesn't have the use-strict babel plugin and that is why it's working.

rossipedia · 2017-06-02T17:35:20Z

I've tested a couple webpack+mobx apps against this branch (as well as with babel-preset-env) with the Object.setPrototypeOf polyfill in place, and things appear to be working correctly. npm run full-test passes. Webpack also reports that mobx.module.js is the file included, instead of mobx.js.

Not sure how much of a blocker that code coverage check is, but it's understandable since in most current environments Object.setPrototypeOf is available and the polyfill code won't be executed.

capaj · 2017-06-03T15:42:00Z

@rossipedia Mobx is supposed to be usable from IE9 and up. Since IE10 and IE9 doesn't have Object.setPrototypeOf not even object.__proto__ I suspect they might get broken by the change from StubArray.prototype = [] hack. Probably best to put down one more if statement to cover those ancient browsers as well. What do you think @rossipedia ?

rossipedia · 2017-06-03T23:35:41Z

@capaj yeah I think that's probably a good way to go. something like this:

diff --git a/src/types/observablearray.ts b/src/types/observablearray.ts
index 9b53ea3..44d04bb 100644
--- a/src/types/observablearray.ts
+++ b/src/types/observablearray.ts
@@ -75,13 +75,19 @@ export interface IArrayWillSplice<T> {
 let OBSERVABLE_ARRAY_BUFFER_SIZE = 0;

 // Typescript workaround to make sure ObservableArray extends Array
-export class StubArray {
+export class StubArray { }
+function inherit(ctor, proto) {
+       if (typeof Object["setPrototypeOf"] !== "undefined") {
+               Object["setPrototypeOf"](ctor.prototype, proto);
+       } else if (typeof ctor.prototype.__proto__ !== "undefined") {
+               ctor.prototype.__proto__ = proto;
+       } else {
+               ctor["prototype"] = proto;
+       }
 }
-const setProto: (o, p) => void = typeof Object["setPrototypeOf"] !== "undefined"
-       ? Object["setPrototypeOf"]
-       : (obj, proto) => obj.__proto__ = proto
-       ;
-setProto(StubArray.prototype, Array.prototype);
+inherit(StubArray, Array.prototype);
+
+

I only tested this in the IE9 emulation mode in IE11 (which should be a fairly accurate emulation), and it seemed to work fine.

coveralls · 2017-06-03T23:57:31Z

Coverage decreased (-0.1%) to 95.661% when pulling fcbaca3 on rossipedia:module-build into 021cdbb on mobxjs:master.

capaj · 2017-06-04T22:21:16Z

@rossipedia yeah emulation is definitely good enough in IE. Fixed the problem I've had before. Great work. Merging.

mweststrate mentioned this pull request May 31, 2017

Compile ES2015 modules with ES5 target #889

Closed

This was referenced May 31, 2017

Native ES module import #1024

Closed

add a "module" entry point to package.json #1026

Closed

capaj approved these changes May 31, 2017

View reviewed changes

remove es2015.promise from tsconfig libs

4537301

- this isn't actually necessary, it's an artifact from when I was putting the build script together in .ts before converting it back to .js

capaj added the 👓 needs investigation label Jun 2, 2017

capaj mentioned this pull request Jun 2, 2017

Cannot assign to read only property 'prototype' of function 'class ... babel/babel#5816

Closed

rossipedia added 2 commits June 2, 2017 10:31

change var to const to make tslint happy

b571192

can't enumerate ES6 class methods using Object.keys, have to use Obje…

63f1496

…ct.getOwnPropertyNames instead

make StubArray extend Array directly

99fd2fa

- babel doesn't like it when we set .prototype - es2015 spec says Array is extendable

rossipedia added 2 commits June 2, 2017 10:58

Revert "make StubArray extend Array directly"

3b376bf

This reverts commit 99fd2fa.

use Object.setPrototypeOf polyfill for StubArray

d313d1c

- doesn't assign to StubArray.prototype anymore - polyfill from MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/setPrototypeOf

capaj removed the 👓 needs investigation label Jun 3, 2017

modify StubArray prototype assign to support IE9

fcbaca3

capaj merged commit 53a89dc into mobxjs:master Jun 4, 2017

rossipedia deleted the module-build branch June 4, 2017 22:38

phiresky mentioned this pull request Jun 9, 2017

3.1.12 breaks webpack build #1039

Closed

andykog mentioned this pull request Jun 28, 2017

mobx from 3.1.12 to 3.1.16 stops work as expected #1066

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add an es-modules build (referenced with pkg.module) #1027

add an es-modules build (referenced with pkg.module) #1027

rossipedia commented May 31, 2017

capaj commented May 31, 2017

coveralls commented May 31, 2017

capaj commented Jun 2, 2017

capaj commented Jun 2, 2017

rossipedia commented Jun 2, 2017

rossipedia commented Jun 2, 2017

coveralls commented Jun 2, 2017

capaj commented Jun 2, 2017

rossipedia commented Jun 2, 2017 •

edited

Loading

capaj commented Jun 3, 2017

rossipedia commented Jun 3, 2017

coveralls commented Jun 3, 2017

capaj commented Jun 4, 2017 •

edited

Loading

add an es-modules build (referenced with pkg.module) #1027

add an es-modules build (referenced with pkg.module) #1027

Conversation

rossipedia commented May 31, 2017

Point of discussion:

capaj commented May 31, 2017

coveralls commented May 31, 2017

capaj commented Jun 2, 2017

capaj commented Jun 2, 2017

rossipedia commented Jun 2, 2017

rossipedia commented Jun 2, 2017

coveralls commented Jun 2, 2017

capaj commented Jun 2, 2017

rossipedia commented Jun 2, 2017 • edited Loading

capaj commented Jun 3, 2017

rossipedia commented Jun 3, 2017

coveralls commented Jun 3, 2017

capaj commented Jun 4, 2017 • edited Loading

rossipedia commented Jun 2, 2017 •

edited

Loading

capaj commented Jun 4, 2017 •

edited

Loading