ssrwpo / ssr

SSR - Router with SSR for Node & Meteor
https://ssrwpo.github.io/ssr/
MIT License
93 stars 16 forks source link

[Meteor 1.4.4.1] startup error #50

Closed s7dhansh closed 7 years ago

s7dhansh commented 7 years ago

If I update demo to meteor 1.4.4.1 (I had to remove ssrwpo:uglifyjs for it), I start getting numerous errors of the following type:

 /imports/routes/Folks/routes/Folk.jsx: undefined === [ Node {                                  
   type: 'VariableDeclaration',                                                                   
   start: 0,                                                                                      
   end: 13,                                                                                       
   loc: SourceLocation { start: [Object], end: [Object] },                                        
   at ImportExportVisitor._getBlockBodyInfo    

How to upgrade meteor without breaking the code?

cbilotta commented 7 years ago

Hello,

What's in your .babelrc ?

s7dhansh commented 7 years ago

I am just using the demo from this repo. So the babelrc is the default one:

{
  "presets": ["es2015", "meteor"],
  "plugins": [
    "transform-class-properties",
    "transform-react-constant-elements",
    "transform-react-inline-elements",
    "transform-react-remove-prop-types"
  ]
}
cbilotta commented 7 years ago

You are using the demo of the master branch right ? What is your OS ?

s7dhansh commented 7 years ago

Yes. Linux. Xubuntu 17.04. Is upgrading to Meteor 1.4.4.1 working for you? If yes then can you kindly upgrade and push a branch for me to check?

cbilotta commented 7 years ago

Tried to update. I have the same problem as you with this version. @PEM-- I need your help on this one! I already have projects running on the develop branch for the latest version of meteor. Not sure why master won't update.

PEM-- commented 7 years ago

@s7dhansh I've upgraded ssrwpo:uglifyjs so that it can work with Meteor 1.4.4.1.

s7dhansh commented 7 years ago

@PEM-- thanks. Meteor moved from uglifyjs to babili for better minification in 1.4.4 (https://github.com/meteor/meteor/blob/devel/History.md). Does using ssrwpo:uglifyjs make sense then? Will it cause problems in future updates?

PEM-- commented 7 years ago

@s7dhansh Actually, they didn't shift completely. They are still using uglify.js and if the minification fails, they are using babili. One downside of using multiple uglifier is that you end up configuring both 🤔 For instance, if you need dead code elimination, you'll have to configure uglify and babili if you're not sure that all your imports can be processed via uglify...

PEM-- commented 7 years ago

@s7dhansh I've updated sswpo:ssr too. The 2.5.0 removes errors that you get since react 15.5.x and its PropTypes externalization. Happy coding.

s7dhansh commented 7 years ago

@PEM-- thank you very much. regarding minification, so when we are using ssrwpo:uglifyjs are we using pure uglifyjs or Meteor's babili backup is kicking in.

PEM-- commented 7 years ago

It's just like Meteor's minifier except that I've added options on uglifyjs. This allows to go a bit further on minification and create specific behavior in development (using react-addons-perf, for instance).

PEM-- commented 7 years ago

You should check on one of your apps and see the difference. kB are shaved off like a nice Webpack setup 😉

s7dhansh commented 7 years ago

@PEM-- ok great! will check.

s7dhansh commented 7 years ago

@PEM-- @iRayzer checked the latest master today, still the same error. is it working for you guys? Tried 1.4.4.1 and 1.4.4.2.

PEM-- commented 7 years ago

Still no pb on OSX. I've juste tested the 1.4.4.2 and it works like a charm.

From what I see in the error log, ImportExportVisitor is a related to reify

s7dhansh commented 7 years ago

Hmm.. so how to get it fixed on Linux? It is a bummer that we are stuck on 1.4.3.2. True, the problem is with reify, and comes with ecmascript 0.7.3, 0.7.2 works fine.

s7dhansh commented 7 years ago

@PEM-- can you kindly help? It is definitely not a Meteor issue, or a pure reify issue as well, because things work fine in my other apps without ssr. When I add ssrwpo:ssr (even without ssrwpo:uglifyjs2, it errs. So the problem should be in the usage of reify somewhere, in one or more of the babel plugins maybe. But I am not able to figure it out.

PEM-- commented 7 years ago

@s7dhansh I'm not able to reproduce this behavior. Try to remove parts of the .babelrc (except transform-class-properties) to pinpoint which transform or preset is causing this reify issue.

s7dhansh commented 7 years ago

@PEM-- Removing plugins did not help (Same error with minimal .babelrc, i.e.

{
  "presets": ["meteor"],
  "plugins": [
  ]
}

I have found out that the issue is caused by export statements, when I am doing modular exports, viz.

export {Utils};

Any further ideas?

PEM-- commented 7 years ago

Apart if Utils isn't declared, I don't see anything that could mess up with Babel's import/export.

One thing that I favor is to use an export default when I export a single variable: https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/prefer-default-export.md

PS: This rule is setup in our eslint configuration.

s7dhansh commented 7 years ago

Hmm.. the code does not have any visible issues, as it works perfectly with Meteor 1.3.2.

Just tried the update on ssrwpo:ssr/demo (had forgotten about it). Similar error on demo/imports/api/Folks/server/fixtures.js, with just this line even:

import Folks from '..';

Error:

/home/sudhanshu/.meteor/packages/ecmascript/.0.7.3.11274ot++os+web.browser+web.cordova/plugin.compile-ecmascript.os/npm/node_modules/meteor/babel-compiler/node_modules/reify/lib/import-export-visitor.js:466:14: /imports/api/Folks/server/fixtures.js: undefined === [ Node
   {
   type: 'VariableDeclaration',
   start: 0,
   end: 10,
   loc: SourceLocation { start: [Object], end: [Object] },

So, basically, the error is on any import statement, and modular exports. :confused:

s7dhansh commented 7 years ago

@PEM-- is there any way you can spawn a linux container and check a meteor update on /demo? Sorry for troubling you, but this bug is really getting me.

PEM-- commented 7 years ago

A collaborator of one of our core contributor uses Linux with success.

PEM-- commented 7 years ago

And I deploy all my apps on Linux as well 😉

s7dhansh commented 7 years ago

Oh ok. So is demo working fine for you guys, after the meteor update? Can you kindly ask that collaborator?

PEM-- commented 7 years ago

@iRayzer Could your Linux guy test the master branch's demo and report back to @s7dhansh ?

cbilotta commented 7 years ago

Will do ASAP.