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

Fine tune babel config #49

Closed
dashed opened this issue Jul 28, 2017 · 33 comments · Fixed by #87
Closed

Fine tune babel config #49

dashed opened this issue Jul 28, 2017 · 33 comments · Fixed by #87
Milestone

Comments

@dashed
Copy link
Contributor

dashed commented Jul 28, 2017

I tried react-autocompletely with a create-react-app project, and attempted to build it (i.e. npm run build), but received this error:

Failed to compile.

static/js/main.9d2e66e4.js from UglifyJs
Unexpected token: punc (,) [./~/react-autocompletely/dist/menu-status.js:60,0][static/js/main.9d2e66e4.js:56374,19]

error Command failed with exit code 1.

I believe babel config would probably need some fine tuning: https://github.com/paypal/react-autocompletely/blob/cfc1ffffe38bac9d89555998add9d5ac2024ea40/.babelrc

@kentcdodds
Copy link
Member

Just to be clear, you tried installing the beta right? The current latest version has nothing 😀

@kentcdodds
Copy link
Member

Oh, nevermind. I misunderstood. You definitely have the beta

@kentcdodds
Copy link
Member

Ah yes, we need to add the transform for trailing commas in function parameter lists and calls. Would you like to do that?

@dashed
Copy link
Contributor Author

dashed commented Jul 28, 2017

I may look at this over the weekend.

I don't mind if someone else makes the PR. 👍

@nitin42
Copy link

nitin42 commented Jul 29, 2017

Try this -

Eject from create-react-app and use babili-webpack-plugin instead of uglify JS (there are some issues with it). It should work! (more info - facebook/create-react-app#984)

Let me know !

@kentcdodds
Copy link
Member

Nah, let's just add the plugin. People shouldn't have to eject to use this library 😀

@kentcdodds
Copy link
Member

Hmmm.... I just realized I'm pretty sure that the transform is already included... I just ran node dist/menu-status.js and didn't get any parse errors 🤔

@kentcdodds
Copy link
Member

Tried it with node 4 too

@nitin42
Copy link

nitin42 commented Jul 29, 2017

Which plugin are you using to minify the src ?

@kentcdodds
Copy link
Member

I just reproduced, sorta:

~/Developer/my-app
😎  $ yarn build
yarn build v0.27.5
$ react-scripts build
Creating an optimized production build...
Failed to compile.

static/js/main.bacf41bb.js from UglifyJs
Unexpected token: punc (,) [./~/react-autocompletely/dist/autocomplete.js:234,0][static/js/main.bacf41bb.js:12803,20]

error Command failed with exit code 1.

Here's the code around that spot:

// ... stuff
   227	          selectItem = this.selectItem,
   228	          selectItemAtIndex = this.selectItemAtIndex,
   229	          selectHighlightedItem = this.selectHighlightedItem;
   230	      var setHighlightedIndex = menu.setHighlightedIndex;
   231	      var highlightedIndex = menu.state.highlightedIndex;
   232	
   233	      return {
   234	        selectedItem, // <-- this line... 🤔
   235	        selectItem,
   236	        selectItemAtIndex,
   237	        selectHighlightedItem,
   238	        highlightedIndex,
   239	        setHighlightedIndex,
// ... etc.

Not sure what's up with it. The comma is totally fine there...

@nitin42
Copy link

nitin42 commented Jul 29, 2017

I will look into this issue @kentcdodds. Will let you know! (any tips on using the setup locally with create-react-app ?)

@nitin42
Copy link

nitin42 commented Jul 29, 2017

Got it working 😄 . Here you go - http://autocomplete-react.surge.sh/

Built with create-react-app. Minified the src with babili, copied dist locally to my custom setup (with create-react-app) and it works!

@kentcdodds
Copy link
Member

Let's see what we need to do so folks don't have to eject from CRA. We definitely shouldn't require that! 😄

@nitin42
Copy link

nitin42 commented Jul 29, 2017

@kentcdodds add babel-preset-babili to .babelrc and you're good to go!

@kentcdodds
Copy link
Member

Oh, you mean add babili to this project? I'd like to avoid that too. Could you identify which plugin in babili is making the impact? I'd rather not minify the dist... It's helpful for debugging...

@nitin42
Copy link

nitin42 commented Jul 29, 2017

umm... we can also yield sourcemaps with babel I think so ?

@kentcdodds
Copy link
Member

Sorry, that wont be good enough. Let's just make it work. It really shouldn't be a problem.

@nitin42
Copy link

nitin42 commented Jul 29, 2017

Ok! As u say but this error is really strange 🤔

@kentcdodds
Copy link
Member

Agreed it is strange. I'm sure there's just one plugin that babili is using that fixes the problem. If we find out which one that is then we might know what the problem is and have a better hope of fixing it.

@nitin42
Copy link

nitin42 commented Jul 29, 2017

I'm on it 👍

@kentcdodds
Copy link
Member

Thanks!

@nitin42
Copy link

nitin42 commented Jul 29, 2017

Ahn... actually I am not getting any build errors.

I imported AutoComplete from ./react-autocompletely/dist and it works without any changes!

@kentcdodds how did you reproduced the errors ? because I am not getting any 😅

@dashed
Copy link
Contributor Author

dashed commented Jul 30, 2017

@nitin42 This should be sufficient: import Autocomplete from 'react-autocompletely'

@kentcdodds kentcdodds modified the milestone: 1.0.0 Aug 4, 2017
@planigan
Copy link

planigan commented Aug 4, 2017

Adding the babili preset does not get rid of this error for me. My original error is different as well:

Unexpected character '`' [./~/downshift/dist/autocomplete.js:72,0][static/js/main.7da67971.js:10817,43]

The code from autocomplete.js line 72:

71-    _this.getItemNodeFromIndex = function (index) {
72:      return _this._rootNode.querySelector(`[data-autocomplete-item-index="${index}"]`);
73-    };

Adding the transform-es2015-template-literals plugin, the error moves further along:

Unexpected token: punc (}) [./~/downshift/dist/autocomplete.js:78,30][static/js/main.a3b2e730.js:10823,48]

The code from the offending area now:

75-    _this.setHighlightedIndex = function () {
76:      var highlightedIndex = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : _this.props.defaultHighlightedIndex;
77-
78:      _this.internalSetState({ highlightedIndex }, function () {
79:        _this.maybeScrollToHighlightedElement(highlightedIndex);
80-      });
81-    };

I thought that might be choking on destructuring, so I added transfform-es2015-destructuring and got the same error and code as above. So, I added babel-preset-es2015 and the build got through autocomplete.js and gave me this error:

Unexpected character '`' [./~/downshift/dist/set-a11y-status.js:21,0][static/js/main.71dd8718.js:11538,18]

The code, back to template literals. But whhhhhy?

13:function setStatus(status) {
14-  var isSameAsLast = statuses[statuses.length - 1] === status;
15-  if (isSameAsLast) {
16-    statuses = [].concat(_toConsumableArray(statuses), [status]);
17-  } else {
18-    statuses = [status];
19-  }
20-  var div = getStatusDiv();
21-  div.innerHTML = `${statuses.map(getStatusHtml).join('')}`;
22-}

So, I don't know if any of that helps, but I don't know what to try next. If you have any ideas, I'd be happy to keep trying.

kentcdodds pushed a commit that referenced this issue Aug 4, 2017
@kentcdodds kentcdodds mentioned this issue Aug 4, 2017
4 tasks
kentcdodds pushed a commit that referenced this issue Aug 4, 2017
@kentcdodds
Copy link
Member

I think I fixed it. Thank you for digging @planigan. That gave me the insight I needed!

@planigan
Copy link

planigan commented Aug 4, 2017

@kentcdodds I tried with your new .babelrc and I get the same error as the final error in my last message.

@kentcdodds
Copy link
Member

Hmmm.... That's a surprise...

@kentcdodds kentcdodds reopened this Aug 4, 2017
@dashed
Copy link
Contributor Author

dashed commented Aug 4, 2017

maybe you need to specify the browser prop? https://github.com/babel/babel-preset-env#targetsbrowsers

@planigan
Copy link

planigan commented Aug 4, 2017

I tried with:

{
  "presets": [
    ["env", {
      "targets": {
        "browsers": ["> 1%"]
      }
    }],
    "react"
  ],
  "plugins": [
    "transform-class-properties",
    "transform-object-rest-spread"
  ]
}

Same error.

@dashed
Copy link
Contributor Author

dashed commented Aug 4, 2017

That's really weird. 🤔

@kentcdodds
Copy link
Member

Why don't we try to add the stage 2 preset 🤷‍♂️

@planigan
Copy link

planigan commented Aug 5, 2017

Ok, this was my bad. I apologize. After getting the error fixed in autocomplete.js, I was still running the build with an && cp dist/autocomplete.js, so the set-a11y-status.js was still the old version. @kentcdodds Your new .babelrc works as intended. Sorry to drag this out.

@kentcdodds
Copy link
Member

Oh good! I'm glad that's been sorted out. Thanks for all your help everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants