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

[1.4.2-rc.4] super does not work in some cases because __proto__ is not set #7956

Closed
macrozone opened this Issue Oct 25, 2016 · 23 comments

Comments

Projects
None yet
7 participants
@macrozone

macrozone commented Oct 25, 2016

Meteor 1.4.2-rc.4 seems to break some npm packages: vazco/uniforms#117

I could not fully track this down, but what I found out is, that the meteor-version of babel-runtime does not set proto / Object.setPrototypeOf

Compare this:
meteor version: https://github.com/meteor/meteor/blob/devel/packages/babel-runtime/babel-runtime.js#L81

with this:
babel version: https://github.com/babel/babel/blob/master/packages/babel-helpers/src/helpers.js#L378

super() is transformed to this:

var _this = (0, _possibleConstructorReturn3['default'])(this, (_class.__proto__ || Object.getPrototypeOf(_class)).call(this, props));

When running under meteor 1.4.2, _class.__proto__ is just an empty anonymous function, whereas under meteor 1.4.1.x _class.__proto__ is set to a constructor-function

It might be also related to the fact, that this particular package uses static properties. I can't imagine, that inheritance just breaks without noticing.

Anyone an idea, what's going on here?

@benjamn

This comment has been minimized.

Member

benjamn commented Oct 25, 2016

I agree this is a bug. Fortunately we can fix it with an update to babel-runtime without doing a whole Meteor release.

@macrozone

This comment has been minimized.

macrozone commented Oct 26, 2016

As 1.4.2 is now released, how can i update babel-runtime separatly?

@busstram

This comment has been minimized.

busstram commented Oct 26, 2016

Is this why my 2x subclassing of Mongo.Collection fails? It says insert or remove are undefined functions.

@neurosoup

This comment has been minimized.

neurosoup commented Oct 26, 2016

Same for me, I am subclassing Mongo.Collection and have error 'insert undefined' since 1.4.2 release. Is the issue is linked ?

@benjamn

This comment has been minimized.

Member

benjamn commented Oct 26, 2016

Here's the original explanation of why we chose not to set __proto__. In short, it wouldn't work in Internet Explorer <10, and any code you wrote that relied on that feature would appear to work in other browsers, but fail in older IEs, where you might not be actively testing.

The good news is that Babel now attempts to work around the limitations of older IEs by using _class.__proto__ || Object.getPrototypeOf(_class) instead of Object.getPrototypeOf(_class), thanks to this commit.

@benjamn

This comment has been minimized.

Member

benjamn commented Oct 26, 2016

I think I know how to fix this, but I'm having trouble reproducing it. Can anyone provide a small example app that has this problem?

@busstram

This comment has been minimized.

busstram commented Oct 26, 2016

I tried to, but I can't get it to not work either in a new project.

In my current project, however, this code:

import { Mongo } from 'meteor/mongo';

export default class Collection extends Mongo.Collection {
  constructor({ name, schema, helpers } = {}) {
    super(name);
    if (schema) this.attachSchema(schema);
    if (helpers) this.helpers(helpers);
  }
}

console.log(Collection);
console.log(new Collection({ name: 'test' }));

yields:

{ [Function: Collection]
I20161027-01:08:55.358(2)?   _publishCursor: [Function],
I20161027-01:08:55.358(2)?   _rewriteSelector: [Function],
I20161027-01:08:55.358(2)?   Cursor: [Function],
I20161027-01:08:55.358(2)?   ObjectID: [Function] }
I20161027-01:08:55.359(2)? Collection {}

package.json deps:

"dependencies": {
    "apollo-client": "^0.4.19",
    "apollo-server": "^0.3.2",
    "body-parser": "^1.15.2",
    "casual": "^1.5.5",
    "classnames": "^2.2.5",
    "cloudinary": "^1.4.3",
    "eslint-config-airbnb": "^12.0.0",
    "express": "^4.14.0",
    "flickity-fix": "^1.1.2",
    "form-data": "^2.1.1",
    "graphql": "^0.7.0",
    "graphql-tools": "^0.7.2",
    "graphql-typings": "0.0.1-beta-2",
    "key-mirror": "^1.0.1",
    "meteor-node-stubs": "~0.2.0",
    "node-fetch": "^1.6.3",
    "normalize.css": "^4.2.0",
    "prerender-node": "^2.1.0",
    "radium": "^0.18.1",
    "react": "^15.3.2",
    "react-apollo": "^0.5.6",
    "react-dom": "^15.3.2",
    "react-helmet": "^3.1.0",
    "react-motion": "^0.4.5",
    "react-redux": "^4.4.5",
    "react-router": "^2.8.1",
    "react-router-transition": "0.0.6",
    "react-scroll": "^1.4.0",
    "redux": "^3.6.0",
    "tinycolor2": "^1.4.1"
  },
  "devDependencies": {
    "babel-eslint": "^7.0.0",
    "babel-plugin-inline-import": "^2.0.4",
    "babel-plugin-syntax-async-functions": "^6.13.0",
    "babel-plugin-transform-async-to-generator": "^6.16.0",
    "babel-preset-es2015": "^6.14.0",
    "babel-preset-react": "^6.11.1",
    "babel-preset-stage-2": "^6.13.0",
    "eslint": "^3.5.0",
    "eslint-config-airbnb": "^11.1.0",
    "eslint-import-resolver-meteor": "^0.3.3",
    "eslint-plugin-import": "^1.16.0",
    "eslint-plugin-jsx-a11y": "^2.2.2",
    "eslint-plugin-meteor": "^4.0.0",
    "eslint-plugin-react": "^6.3.0"
  }

.meteor/packages:

meteor-base@1.0.4
mobile-experience@1.0.4
mongo@1.1.14
reactive-var@1.0.11
tracker@1.1.1

standard-minifier-css@1.3.2
standard-minifier-js@1.2.1
es5-shim@4.6.15
ecmascript@0.5.9
shell-server@0.2.1
static-html
apollo
dburles:collection-helpers
aldeed:simple-schema
meteorhacks:picker
aldeed:collection2
jabbslad:basic-auth
force-ssl@1.0.13
meteorhacks:kadira
random@1.0.10

.meteor/versions:

accounts-base@1.2.14
aldeed:collection2@2.10.0
aldeed:collection2-core@1.2.0
aldeed:schema-deny@1.1.0
aldeed:schema-index@1.1.0
aldeed:simple-schema@1.5.3
allow-deny@1.0.5
apollo@0.1.2
autoupdate@1.2.11
babel-compiler@6.13.0
babel-runtime@0.1.12
base64@1.0.10
binary-heap@1.0.10
blaze@2.1.9
blaze-tools@1.0.10
boilerplate-generator@1.0.11
caching-compiler@1.1.8
caching-html-compiler@1.0.7
callback-hook@1.0.10
check@1.2.4
dburles:collection-helpers@1.1.0
ddp@1.2.5
ddp-client@1.2.9
ddp-common@1.2.7
ddp-rate-limiter@1.0.6
ddp-server@1.2.10
deps@1.0.12
diff-sequence@1.0.7
ecmascript@0.5.9
ecmascript-runtime@0.3.15
ejson@1.0.13
email@1.0.16
es5-shim@4.6.15
fastclick@1.0.13
force-ssl@1.0.13
geojson-utils@1.0.10
hot-code-push@1.0.4
html-tools@1.0.11
htmljs@1.0.11
http@1.1.8
id-map@1.0.9
jabbslad:basic-auth@0.2.2
jquery@1.11.10
launch-screen@1.0.12
livedata@1.0.18
localstorage@1.0.12
logging@1.1.16
mdg:validation-error@0.2.0
meteor@1.6.0
meteor-base@1.0.4
meteorhacks:kadira@2.30.2
meteorhacks:meteorx@1.4.1
meteorhacks:picker@1.0.3
minifier-css@1.2.15
minifier-js@1.2.15
minimongo@1.0.18
mobile-experience@1.0.4
mobile-status-bar@1.0.13
modules@0.7.7
modules-runtime@0.7.7
mongo@1.1.14
mongo-id@1.0.6
mongo-livedata@1.0.12
npm-mongo@2.2.11_2
observe-sequence@1.0.14
ordered-dict@1.0.9
promise@0.8.8
raix:eventemitter@0.1.3
random@1.0.10
rate-limit@1.0.6
reactive-var@1.0.11
reload@1.1.11
retry@1.0.9
routepolicy@1.0.12
service-configuration@1.0.11
shell-server@0.2.1
spacebars@1.0.13
spacebars-compiler@1.0.13
standard-minifier-css@1.3.2
standard-minifier-js@1.2.1
static-html@1.1.13
templating-tools@1.0.5
tmeasday:check-npm-versions@0.3.1
tracker@1.1.1
ui@1.0.12
underscore@1.0.10
url@1.0.11
webapp@1.3.12
webapp-hashing@1.0.9

@benjamn benjamn added this to the Release 1.4.2.1 milestone Oct 26, 2016

@busstram

This comment has been minimized.

busstram commented Oct 27, 2016

FYI: I also added my packages file.

@macrozone

This comment has been minimized.

macrozone commented Oct 27, 2016

@benjamn: so why was it working in previous versions of meteor? I never checked on IE versions below 10, was it just not working there?

I am also not sure, if it is a good decision to make a regression for all browsers, because it does not work on a IE < 10...

anyway, I hope 1.4.2.1 will land soon, that we can profit from the awesome performance tweaks :-)

I try to setup a reproduction...

@macrozone

This comment has been minimized.

macrozone commented Oct 27, 2016

ok @benjamn , here is a reproduction based on uniforms's demo-project.

I added a custom form-theme that uses static classes along the themes installed from npm. (mine is a uncompiled copy from uniforms-unstyled)

https://github.com/macrozone/meteor-1.4.2-static-inheritance-reproduction

clone, npm install and start it. It's running meteor 1.4.1.

Then on the page, you can select the form-themes. Every theme including my local "custom"-theme should work.

Then update to meteor 1.4.2.

~~now, the custom-form will not work anymore. (will throw an error in the console). ~~

On my first try, only my custom-form did not work anymore. But now, as i am writing this issues, all theme do not work anymore under 1.4.2 due to the same problem.

hope this helps.

@benjamn

This comment has been minimized.

Member

benjamn commented Oct 27, 2016

@macrozone It worked in previous versions of Meteor because we just copied static properties, which worked equally well in all browsers. What changed was a newer version of Babel assuming that __proto__ had been set.

@benjamn

This comment has been minimized.

Member

benjamn commented Oct 27, 2016

Ok, after digging into that reproduction (thanks @macrozone!), it looks like the source of the problem is code in node_modules that wasn't compiled by Meteor, such as AutoForm.js. I think this code worked before 1.4.2 because it was able to use node_modules/babel-runtime/helpers/inherits.js, whereas 1.4.2 ignores the babel-runtime helpers that are provided by babel-runtime.js, and the version of inherits that it uses doesn't set __proto__.

Short-term, making sure babel-runtime.js matches the behavior of the babel-runtime npm package regarding __proto__ should fix the immediate problem.

Longer term, we need to evaluate whether implementing Babel helpers in a Meteor specific way still has its original values: minimizing the use of unnecessary runtime libraries, and supporting older browsers. If babel-runtime meets those requirements now (or comes close enough), then the Meteor babel-runtime package should probably go away and babel-runtime should become part of every app's package.json.

@benjamn

This comment has been minimized.

Member

benjamn commented Oct 28, 2016

This should be fixed if you run meteor update --release 1.4.2.1-beta.0 in your app directory.

@macrozone

This comment has been minimized.

macrozone commented Oct 31, 2016

ok, i updated to 1.4.2.1-beta.0, but the problem still exists. Do i need to meteor reset, wipe node_modules or similar?

@busstram

This comment has been minimized.

busstram commented Oct 31, 2016

Might be your build cache.
On Mon, 31 Oct 2016 at 09:49, Marco Wettstein notifications@github.com
wrote:

ok, i updated to 1.4.2.1-beta.0, but the problem still exists. Do i need
to meteor reset, wipe node_modules or similar?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#7956 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFRpBsED25nPnOmZy5AaShhJZCnbCupbks5q5auygaJpZM4Kf4Dd
.

@sakulstra

This comment has been minimized.

Contributor

sakulstra commented Oct 31, 2016

I think this is somehow related - also got the error after upgrading to 1.4.2

What I did now is:
meteor update --release 1.4.2.1-beta.0
meteor reset
rm node_modules -r && meteor npm install
meteor npm run start

I still get the same uniforms error
Warning: Failed childContext type: Required child contextuniforms.modelwas not specified inAutoValidatedQuickUnstyledForm.

@radekmie

This comment has been minimized.

Contributor

radekmie commented Oct 31, 2016

@sakulstra
Yes, it's related, because Auto overrides getChildContextModel, so it pass the model from the state, but it's not set, because super in constructor of Unstyled is noop.

@benjamn
Unluckily I have to agree with @macrozone - even after clearing node_modules and doing meteor reset, it's still not working in 1.4.2.1-beta.0.

@mimamuh

This comment has been minimized.

mimamuh commented Oct 31, 2016

Me too, doesn't work after updating to 1.4.2.1-beta.0. Can't run a classes as the constructor won't be executed.

@busstram

This comment has been minimized.

busstram commented Oct 31, 2016

Mine worked after deleting node_modules and .meteor/local.
On Mon, 31 Oct 2016 at 18:06, Michael Neumair notifications@github.com
wrote:

Me too, doesn't work after updating to 1.4.2.1-beta.0. Can't run a
classes as the constructor won't be executed.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#7956 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFRpBkKIIC8k3gipGM5dkyCP3YCi_jSTks5q5iAWgaJpZM4Kf4Dd
.

@benjamn benjamn closed this in 9dfcc38 Oct 31, 2016

benjamn added a commit that referenced this issue Oct 31, 2016

Add babel-runtime to the default new-app package.json file.
In light of issues like #7956, I would very much like for app developers
to be responsible for providing node_modules/babel-runtime, and for the
Meteor babel-runtime package to stop attempting to implement a subset of
the helpers.

benjamn added a commit that referenced this issue Oct 31, 2016

Simplify Meteor babel-runtime package by relying on npm version.
Though this may seem like a significant change, this package will still
work for older apps as long as the developer follows the instructions
about installing the babel-runtime npm package.

Helps with #7956.

@benjamn benjamn reopened this Oct 31, 2016

@benjamn

This comment has been minimized.

Member

benjamn commented Oct 31, 2016

My commit 9dfcc38 might be enough to fix this issue in combination with reinstalling node_modules and/or removing .meteor/local, but I think we can do better than that, so I'm reopening this.

@busstram

This comment has been minimized.

busstram commented Oct 31, 2016

Perhaps doing a build-cache clear post release update would suffice?

@macrozone

This comment has been minimized.

macrozone commented Nov 1, 2016

@benjamn clearing .meteor/local did not help or instlaling node_modules.

instead, i had to bump version in .meteor/versions of babel-runtime to babel-runtime@0.1.13

benjamn added a commit that referenced this issue Nov 2, 2016

Add babel-runtime to the default new-app package.json file.
In light of issues like #7956, I would very much like for app developers
to be responsible for providing node_modules/babel-runtime, and for the
Meteor babel-runtime package to stop attempting to implement a subset of
the helpers.

benjamn added a commit that referenced this issue Nov 2, 2016

Simplify Meteor babel-runtime package by relying on npm version.
Though this may seem like a significant change, this package will still
work for older apps as long as the developer follows the instructions
about installing the babel-runtime npm package.

Helps with #7956.
@benjamn

This comment has been minimized.

Member

benjamn commented Nov 3, 2016

Meteor 1.4.2.1-beta.1 contains some noteworthy changes to the way babel-runtime works, thanks to #7995. Please try the reproduction steps again after running meteor update --release 1.4.2.1-beta.1 and meteor npm install --save babel-runtime.

Closing now because the problem is fixed for me, but please reopen if you discover otherwise.

@benjamn benjamn closed this Nov 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment