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

Package presence causes SyntaxError in CI #4

Closed
alexlafroscia opened this issue Jan 30, 2019 · 13 comments · Fixed by emberjs/ember-cli-babel#267
Closed

Package presence causes SyntaxError in CI #4

alexlafroscia opened this issue Jan 30, 2019 · 13 comments · Fixed by emberjs/ember-cli-babel#267

Comments

@alexlafroscia
Copy link

alexlafroscia commented Jan 30, 2019

I am trying to include this package as a dependency of an addon that I'm working on, but it's throwing off my build in CI.

Locally, the tests in this branch pass at this commit:

alexlafroscia/ember-rx@e92ea73

However, in CI, I get a SyntaxError before Chrome can actually run any of my tests.

https://travis-ci.com/alexlafroscia/ember-rx/jobs/173939015#L544

I added an additional commit to the one that works locally, which removes the polyfill; no more SyntaxError (the build fails, though, because I need the polyfill to run my tests!)

https://travis-ci.com/alexlafroscia/ember-rx/jobs/173939684

Any idea what's going on here?

@rwjblue
Copy link
Member

rwjblue commented Jan 30, 2019

We need to know whats at /assets/vendor.js line 56181 to have an idea about path forward.

I think you should be able to replicate locally with CI=true ember test, so you can poke at what the invalid identifier is there...

@alexlafroscia
Copy link
Author

Cool — I tried doing a build locally yesterday and loaded up that file, but there wasn’t anything fishy on that line.

I’ll try that locally with the CI flag and let you know what I find!

@alexlafroscia
Copy link
Author

Sure enough, CI=true ember test failed locally.

I ran this:

CI=true yarn build --environment test

to get the file in a location where I could open it. Here are the lines directly above/below it

}());

;import _taggedTemplateLiteral from "@babel/runtime/helpers/esm/taggedTemplateLiteral";
import _possibleConstructorReturn from "@babel/runtime/helpers/esm/possibleConstructorReturn";
import _getPrototypeOf from "@babel/runtime/helpers/esm/getPrototypeOf";

The line where _taggedTemplateLiteral is imported is the one that throws the SyntaxError, which to me makes sense since there are, syntactically, two ; in a row!

I'm not sure what source file that is from -- the module above it (where the }()); is) is rsvp. It's a bit "much" but here's the whole file (zipped, since Github doesn't allow uploads of .js files):

vendor.js.zip

@rwjblue
Copy link
Member

rwjblue commented Feb 3, 2019

Seems like this is an odd interaction between us using compileModules: false AND the new (from emberjs/ember-cli-babel#251) changes. Can you follow the steps in https://github.com/babel/ember-cli-babel#external-helpers to disable the external helpers to confirm that is the culprit?

/cc @pzuraq

@alexlafroscia
Copy link
Author

alexlafroscia commented Feb 4, 2019

Can you follow the steps in https://github.com/babel/ember-cli-babel#external-helpers to disable the external helpers to confirm that is the culprit?

I took this to mean adding the following to the add-on's ember-cli-build.js file:

    "ember-cli-build": {
      includeExternalHelpers: false
    }

Running CI=true ember test still caused the same error after that change.

For the record, hard-coding to true didn't fix the problem, either.

@rwjblue
Copy link
Member

rwjblue commented Feb 5, 2019

Ya, thats what I was thinking. Hmph.

@willviles
Copy link

willviles commented Feb 9, 2019

I'm experiencing this issue too when trying to do a production build with @ember/render-modifiers installed. Inspecting the build vendor.js, it's failing on the same imports as @alexlafroscia, which just don't seem to get compiled regardless of the ember-cli-babel configuration in this addon.

import _taggedTemplateLiteral from"@babel/runtime/helpers/esm/taggedTemplateLiteral"
import _possibleConstructorReturn from"@babel/runtime/helpers/esm/possibleConstructorReturn"
import _getPrototypeOf from"@babel/runtime/helpers/esm/getPrototypeOf"
import _get from"@babel/runtime/helpers/esm/get"
import _inherits from"@babel/runtime/helpers/esm/inherits"
import _classCallCheck from"@babel/runtime/helpers/esm/classCallCheck"
import _createClass from"@babel/runtime/helpers/esm/createClass"

Not sure how to debug this further!

@rwjblue
Copy link
Member

rwjblue commented Feb 11, 2019

I have an idea that I'd like to try, but I'm not sure exactly how to reproduce the issue. Does anyone have repro steps handy?

@willviles
Copy link

willviles commented Feb 11, 2019

I've put together a barebones reproduction at willviles/ember-modifier-manager-polyfill-bug.

It's got Prember installed so it throws an error when the production build's vendor.js output isn't transpiled properly.

One thing I've noticed, uninstalling ONE of @ember-decorators/babel-transforms OR @ember/render-modifiers seems to fix the build 🤷‍♂️

Having both installed at the same time causes the SyntaxError to rear its ugly head.

@rwjblue
Copy link
Member

rwjblue commented Feb 11, 2019

Yep, thats along the lines of my hypothesis. Thank you for the repro!

@rwjblue
Copy link
Member

rwjblue commented Feb 12, 2019

OK, awesome, I've tracked this down and filed a PR to fix the issue over in emberjs/ember-cli-babel#267.

@rwjblue
Copy link
Member

rwjblue commented Feb 12, 2019

OK, released v1.0.2 which bumps the minimum ember-cli-babel version to fix the underlying issue.

@willviles - I confirmed in your demo repo that just bumping ember-modifier-manager-polyfill to 1.0.2 fixes.

@alexlafroscia
Copy link
Author

Awesome, thanks for the reproduction @willviles and for hunting down the fix @rwjblue !

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.

3 participants