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

How do I enable private methods? #979

Closed
NullVoxPopuli opened this issue Sep 24, 2021 · 10 comments
Closed

How do I enable private methods? #979

NullVoxPopuli opened this issue Sep 24, 2021 · 10 comments

Comments

@NullVoxPopuli
Copy link
Collaborator

NullVoxPopuli commented Sep 24, 2021

I'm surprised they aren't enabled by default? because ember-cli-babel has them 🙃

but anywho, I have this error in C.I.: https://github.com/NullVoxPopuli/ember-array-map-resource/pull/103/checks?check_run_id=3702630696#step:5:215

 - originalErrorMessage: /home/runner/work/ember-array-map-resource/ember-array-map-resource/-private/resources/array-map.ts: Class private methods are not enabled.
  20 |
  21 |   // @private
> 22 |   get #records(): Element[] {
     |   ^
  23 |     return this.args.positional[0];
  24 |   }
  25 |
  - stack: SyntaxError: /home/runner/work/ember-array-map-resource/ember-array-map-resource/-private/resources/array-map.ts: Class private methods are not enabled.

Classic build works just fine with private fields: https://github.com/NullVoxPopuli/ember-array-map-resource/pull/103/checks?check_run_id=3702630588

@ef4
Copy link
Contributor

ef4 commented Sep 26, 2021

I can't reproduce this. I tested private fields working in both an app and an addon. Maybe you have old dependencies (like a @babel/preset-env) in a lockfile?

@NullVoxPopuli
Copy link
Collaborator Author

NullVoxPopuli commented Sep 26, 2021

I re-rolled the lockfile and got the same issue: https://github.com/NullVoxPopuli/ember-array-map-resource/pull/106/checks?check_run_id=3714144862#step:5:215

I think the issue is more private methods rather than private fields -- I'm surprised these are different implementation-wise in Babel.

But yeah, private fields already work just fine.

What's goofy is that private methods are natively supported,
image

though, my targets currently include safari: https://github.com/NullVoxPopuli/ember-array-map-resource/blob/main/config/targets.js#L3

I wonder if that's why transpilation is even occurring.

I've added a private getter here: https://github.com/embroider-build/embroider/pull/982/files to see if the test suite passes with this addition

@NullVoxPopuli NullVoxPopuli changed the title How do I enable private fields? How do I enable private methods? Oct 2, 2021
@NullVoxPopuli
Copy link
Collaborator Author

ember-cli-babel has private methods here: https://github.com/babel/ember-cli-babel/blob/085c408b88a058bdbcaa298a5e59c9511e832073/lib/babel-options-util.js#L403
but only in the else condition. 🤔

I searched embroider for @babel/plugin-proposal-private-methods but it couldn't be found. 🤔
I shall add it!

@courajs
Copy link

courajs commented Nov 16, 2021

Been running into this as well. Discovered @embroider/webpack depends on the old pre-release version of babel's preset env. babel-preset-env has been deprecated in favor of @babel/preset-env.
Looks like the new versions use plugin-proposal-private-methods: https://github.com/babel/babel/blob/87fc2e76d77b0bc78b76cced469bd64c5edf141f/packages/babel-preset-env/package.json#L37
But the pre-release package doesn't: https://github.com/babel/babel-preset-env/blob/31f33e3656e1bd8036317c54128fdc2812a2e8aa/package.json#L24

So I believe the solution is to migrate from babel-preset-env to @babel/preset-env

@rwjblue
Copy link
Collaborator

rwjblue commented Nov 17, 2021

@courajs - I don't think we are using that dep at all actually...

@rwjblue
Copy link
Collaborator

rwjblue commented Nov 17, 2021

I believe this is the same thing I described in emberjs/ember-cli-babel#419 (comment), since we use ember-cli-babel for compilation still we're going to have the same issue.

@ef4
Copy link
Contributor

ef4 commented Dec 20, 2021

I suspect this is fixed by emberjs/ember-cli-babel#420

@ef4 ef4 closed this as completed Dec 20, 2021
@NullVoxPopuli
Copy link
Collaborator Author

I'll double check real quick

@NullVoxPopuli
Copy link
Collaborator Author

@NullVoxPopuli
Copy link
Collaborator Author

NullVoxPopuli commented Dec 20, 2021

ok, too much going on here, there is the minimal test:
NullVoxPopuli/ember-array-map-resource#212

and it succeeded! yay!
confirm fixed.

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

4 participants