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

Does not work with ember-source 3.27+ #70

Closed
mydea opened this issue Apr 26, 2021 · 16 comments
Closed

Does not work with ember-source 3.27+ #70

mydea opened this issue Apr 26, 2021 · 16 comments

Comments

@mydea
Copy link
Contributor

mydea commented Apr 26, 2021

On 3.27-beta.4, I get the error:

Uncaught SyntaxError: Cannot use import statement outside a module

Screenshot from 2021-04-26 14-43-00

mydea added a commit to mydea/ember-date-components that referenced this issue Apr 26, 2021
ember-cached-decorator-polyfill has a bug with those versions:
ember-polyfills/ember-cached-decorator-polyfill#70
@boris-petrov
Copy link

I just hit the same issue when upgrading to Ember 3.27.

@rwjblue, @buschtoens - what should we do?

@rwjblue
Copy link
Member

rwjblue commented May 5, 2021

This is basically because of

treeForVendor(tree) {
const babel = this.addons.find(a => a.name === 'ember-cli-babel');
return babel.transpileTree(tree, {
babel: this.options.babel,
'ember-cli-babel': {
compileModules: false
}
});
},

Specifically, we are telling ember-cli-babel that it shouldn't compile modules. But that isn't going to work quite right (since in Ember 3.27+ the modules are actually directly used).

@rwjblue
Copy link
Member

rwjblue commented May 5, 2021

Hmm. I'm not able to reproduce (I checked out this repo and ran its tests). Can someone make a quick reproduction so I can debug?

@boris-petrov
Copy link

boris-petrov commented May 5, 2021

@rwjblue - it's as simple as:

ember new test-test
cd test-test
# modify package.json to use Ember 3.27
npm install
ember install ember-cached-decorator-polyfill
ember test -s

@rwjblue
Copy link
Member

rwjblue commented May 6, 2021

I think emberjs/ember-cli-babel#402 will resolve.

@rwjblue
Copy link
Member

rwjblue commented May 6, 2021

@boris-petrov
Copy link

@rwjblue - the same example I gave above still doesn't work even with 7.26.5. :( My project also doesn't work.

@knownasilya
Copy link

knownasilya commented May 7, 2021

Using ember-cli-babel v7.26.5 still seeing the issue.

@runspired
Copy link
Contributor

runspired commented May 10, 2021

We're also hitting this in EmberData with ember-cli-babel 7.26.5. I noticed this was still resolving an older version though from the lockfile and so I regenerated it and made sure it was resolving 7.26.5. I've confirmed it is still broken even when this addon does resolve 7.26.5, so the fix upstream was insufficient.

@ef4
Copy link
Contributor

ef4 commented May 10, 2021

I don't think emberjs/ember-cli-babel#402 is complete or correct, see emberjs/ember-cli-babel#402 (comment)

This addon is using import along with compileModules: false. That is a contradiction. It worked by accident on earlier Ember versions, it doesn't work anymore.

As long as compileModules: false is set, you can't author as an ES module, so you can't use import. The simplest fix is to use the Ember global instead, and yes, that will trigger immediate deprecations.

A more complete fix is to refactor this polyfill so it transpiles import { cached } from '@glimmer/tracking' to import { cached } from 'ember-cached-decorator-polyfill' and provides its implementation in treeForAddon instead of treeForVendor, so it's all proper ES modules.

@mydea
Copy link
Contributor Author

mydea commented May 12, 2021

I made a PR with a refactor to using the approach proposed by @ef4 here: #75

@rwjblue
Copy link
Member

rwjblue commented May 18, 2021

I paired with @ef4 to make the changes in ember-cli-babel more correct (well, really they are still kinda bonkers but they more correctly emulate the "old world") in emberjs/ember-cli-babel#407.

Testing the reproduction steps provided by @boris-petrov results in a passing test run.

@rwjblue rwjblue closed this as completed May 18, 2021
@boris-petrov
Copy link

boris-petrov commented May 19, 2021

@rwjblue - thanks! That fixed the error from above... but it still doesn't work. 😄

Use the reproduction from above and just add in app.js:

import { cached } from '@glimmer/tracking';

class X {
  @cached
  get foo() {
    return 1;
  }
}

Then running ember test leads to:

Uncaught TypeError: decorator is not a function
    at applyDecoratedDescriptor.js:22
    at Array.reduce (<anonymous>)
    at _applyDecoratedDescriptor (applyDecoratedDescriptor.js:21)
    at Module.callback (store.js:868)
    at Module.exports (loader.js:106)
    at Module._reify (loader.js:143)
    at Module.reify (loader.js:130)
    at Module.exports (loader.js:104)
    at Module._reify (loader.js:143)
    at Module.reify (loader.js:130)

@acorncom
Copy link

acorncom commented May 19, 2021

Confirmed that I'm seeing the same issue as @boris-petrov. Looks like the haxx introduced in #12 may be causing snags here? Reproduction available here: https://github.com/acorncom/cached-decorator-babel-issues-reproduction (latest ember-cli-babel@7.26.6 in use)

In my case, the error thrown is:

applyDecoratedDescriptor.js:16 Uncaught TypeError: decorator is not a function
    at eval (applyDecoratedDescriptor.js:16)
    at Array.reduce (<anonymous>)
    at _applyDecoratedDescriptor (applyDecoratedDescriptor.js:15)
    at eval (test-component.js:49)
    at Module../components/test-component.js (chunk.d76dcaa29bf3c00d6f74.js:92)
    at __webpack_require__ (chunk.7e29ea896c9f6f20b6e2.js:80)
    at Module.eval [as callback] (module-issues.js:70)
    at Module.exports (loader.js:106)
    at requireModule (loader.js:27)
    at Class._extractDefaultExport (index.js:466)

and that seems to be causing issues for ember-page-title.

@acorncom
Copy link

acorncom commented May 19, 2021

Issue opened at #77

@rwjblue
Copy link
Member

rwjblue commented May 19, 2021

I landed @mydea's PR and released in https://github.com/ember-polyfills/ember-cached-decorator-polyfill/releases/tag/v0.1.2. I also confirmed in @acorncom's reproduction (from #77) works without issue with that update (tested against both Embroider and non-Embroider builds).

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

No branches or pull requests

7 participants