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

You may need an appropriate loader to handle this file type - when ES6 classes contain public or private field declarations #10216

Closed
alan-agius4 opened this issue Jan 5, 2020 · 45 comments

Comments

@alan-agius4
Copy link

What is the current behavior?
When using public/private field declarations in an es6 class. Webpack fails to bundle with the below error

ERROR in ./index.js 2:9
Module parse failed: Unexpected token (2:9)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| class Rectangle {
>   height = 0;
|   width;
| 


ERROR in ./index.js 10:2
Module parse failed: Unexpected character '#' (10:2)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| 
| class RectangleP {
>   #height = 0;
|   #width;
|   constructor(height, width) {

If the current behavior is a bug, please provide the steps to reproduce.

index.js

class Rectangle {
  height = 0;
  width;

  constructor(height, width) {
    this.height = height;
    this.width = width;
  }
}

class RectangleP {
  #height = 0;
  #width;
  constructor(height, width) {
    this.#height = height;
    this.#width = width;
  }
}

webpack.config.js

const { resolve } = require('path');

module.exports = {
    entry: './index.js',
    output: {
      path: resolve(__dirname, 'dist'),
      filename: '[name].js'
    },
    resolve: {
        extensions: [ '.js' ]
    }
}

What is the expected behavior?
Code gets bundled.

Other relevant information:
webpack version: 4.41.5
Node.js version: v12.12.0
Operating System: macOS Catalina
Additional tools:

@alan-agius4 alan-agius4 changed the title When ES6 classes contain public or private fields You may need an appropriate loader to handle this file type - when ES6 classes contain public or private fields Jan 5, 2020
@alan-agius4 alan-agius4 changed the title You may need an appropriate loader to handle this file type - when ES6 classes contain public or private fields You may need an appropriate loader to handle this file type - when ES6 classes contain public or private Field declarations Jan 5, 2020
@alan-agius4 alan-agius4 changed the title You may need an appropriate loader to handle this file type - when ES6 classes contain public or private Field declarations You may need an appropriate loader to handle this file type - when ES6 classes contain public or private field declarations Jan 5, 2020
@MarkH817
Copy link

MarkH817 commented Jan 5, 2020

https://github.com/tc39/proposal-class-fields
Class fields syntax is currently at stage 3, so you may need to transpile it using babel.

You'd need to add this plugin to your babel config: https://babeljs.io/docs/en/babel-plugin-proposal-class-properties

@alan-agius4
Copy link
Author

In my case I don’t want to downlevel since node 12 target does accept these fields are available natively

@sokra
Copy link
Member

sokra commented Jan 5, 2020

We are using acorn (https://github.com/acornjs/acorn) as parser and it only support stage 4 proposals.

@inoyakaigor
Copy link

@sokra is there a way to skip parse a code? I think my problem #10227 is the same.

@sokra
Copy link
Member

sokra commented Jan 11, 2020

yep with the module.noParse option, but this also disables import/export in your code, so probably doesn't make sense.

Maybe using https://github.com/acornjs/acorn-stage3 in webpack makes sense, at least opt-in.

@sokra
Copy link
Member

sokra commented Jan 11, 2020

Or the ability to add acorn plugins

@dominiklessel
Copy link

yep with the module.noParse option, but this also disables import/export in your code, so probably doesn't make sense.

Maybe using https://github.com/acornjs/acorn-stage3 in webpack makes sense, at least opt-in.

This would be perfect. This little problem also breaks: -> vercel/ncc#499

Personally I could just stay aways from public or private field declarations, but some of my dependencies (namely hapi.js) are already using it …

@mheiber
Copy link

mheiber commented Feb 14, 2020

acornjs/acorn#830 this seems to be the acorn issue

@noinkling
Copy link

noinkling commented Feb 23, 2020

Maybe using https://github.com/acornjs/acorn-stage3 in webpack makes sense, at least opt-in.

Or the ability to add acorn plugins

Yes please. I've been bitten by this at least a few times now, because I always forget that just because Babel can parse something, it doesn't mean Webpack can (why don't you guys use the same parser again?).

In the relevant case my process went something like this:

  1. Notice that my installed version of Chrome supports class fields now.
  2. Smartly replace plugin-proposal-class-properties with plugin-syntax-class-properties in my Babel dev config, since I know the proposal is stage 3, meaning the Babel parser will still need the syntax plugin to parse it.
  3. Get a parsing error.
  4. Spend a while wondering how that can possibly be, and trying to debug my Babel config.
  5. Finally realise I'm looking at a Webpack/Acorn error not a Babel one (mostly my fault, but it's not exactly clear at a glance).
  6. Revert.
  7. Vow never to make the same mistake again (but inevitably do so anyway a year or so down the road with some other language feature).

@alexander-akait
Copy link
Member

I think it can be solved in a more comfortable way:

  • Allow to set plugins for acorn (first PR)
  • Resolved minimum supported node using browserslist/package.json and setup plugins by default (second PR)

@wycats
Copy link

wycats commented May 8, 2020

Is there any progress on this feature? I hit this problem today. One reason to want to use native private fields is that the debugging experience of transpiled private fields is pretty painful.

@alexander-akait
Copy link
Member

@wycats no, feel free to send a PR, it should be easy

@mforman1
Copy link

mforman1 commented Jun 11, 2020

@sokra Acorn released today a new version that supports optional chaining.
Any idea when this will be updated on your side?

@OnurGvnc
Copy link

OnurGvnc commented Jul 3, 2020

as a temporary solution;
https://github.com/OnurGvnc/acorn-with-stage3

package.json:

{
  ...
  "resolutions": {
    "acorn": "npm:acorn-with-stage3"
  },
  "dependencies": {

@lostpebble
Copy link

Just ran into this today, which was strange because I never had the issue before. Turns out my current babel config was missing a plugin I had been using over the past couple years but forgot to copy over. I added that back in and it worked again.

So as another temp fix, add this to your babel config, inside plugins: []

["@babel/plugin-proposal-class-properties", { loose: true }],

@tobiu
Copy link

tobiu commented Sep 30, 2020

I need public class fields as well for the neo.mjs framework.

In general, we should re-think the approach to only add stage 4 proposals. While this was fine 1-2 years ago, these days most browser engines already implement the important stage 3 ones.

Let's take a look at public class fields:
Screenshot 2020-09-30 at 13 37 37

At this point, even Opera & Safari support them.

For neo.mjs, there is a design goal to use code which runs directly in all major browsers, resulting in not using Babel at all.

Without the ability to specify acorn plugins as webpack configs, I can not add class fields into the code base, since it breaks the dist builds.

I did try @OnurGvnc 's workaround. While this might work for yarn (not tested), it did not work for me using npm.

In short: https://github.com/rogeriochaves/npm-force-resolutions is using a preinstall hook, which is already a problem, in case the package-lock.json is included inside the .gitignore. Without this file, the preinstall will break.

Well, I could add it into the repo, but even then it did not work for me. The script itself did adjust the acorn dependencies inside the package-lock correctly, but the following npm install reverted the changes and webpack was using the default acorn package again. Maybe I am missing something.

Right now the only thing I can think of what I could do is to fork webpack and create an own npm package just to change the acorn dependency. This feels wrong on too many levels (starting with new webpack versions).

I am hoping that we can get the public class fields included soon (should be easy => just add the acorn-with-stage3 plugin). In case there are reasonable workarounds, please let me know.

Best regards,
Tobias

trevtrich added a commit to trevtrich/modules-talk that referenced this issue Oct 9, 2020
@PeteX
Copy link

PeteX commented Oct 20, 2020

I managed to make this work but it's a bit of a hack. First of all, install acorn-class-fields:

npm i --save-dev acorn-class-fields

Next, put this at the top of webpack.config.js:

const classFields = require('acorn-class-fields');
const acorn = require('acorn');
acorn.Parser = acorn.Parser.extend(classFields);

It's important to put this block before you require anything to do with webpack. If you put it further down in the file, webpack will already have created the parser and it won't work.

@NathanaelA
Copy link

FYI: This is now fixed in at least the current version of Webpack 5.35.1, the underlying acorn parser was recently upgraded to 8.2.x which adds Private/public fields to the parser, and webpack 5 is already set to pull the newest 8.x acorn parser. As long as your ecmaVersion is set to 13 or latest (which latest appears to be default) then class fields should work.


I tested by: (Created new test folder and moved to it)

  1. npm i webpack@latest (Installed 5.35.1)
  2. (manually checked node_modules/acorn/package.json -- verified it was >= 8.2.1)
  3. Copied the webpack.config.js & index.js file from first issue and added to the bottom of the index.js file:
    (Otherwise webpack will actually strip it out. because it isn't used. :-D )
let x = new Rectangle(1,1);
let y = new RectangleP(1,1);
console.log(x,y);
  1. webpack
  2. cd dist (Manually looked at output main.js file and verify it was correct)
  3. node main
    Output (as expected) was:
    image

@tobiu
Copy link

tobiu commented Apr 25, 2021

Hi @NathanaelA,

just tested this and it does look really good!

Screenshot 2021-04-25 at 10 27 11

Screenshot 2021-04-25 at 10 22 42

Screenshot 2021-04-25 at 10 25 50

@muzuiget
Copy link

muzuiget commented Apr 25, 2021

I remove the dependency package @babel/plugin-proposal-class-properties in my program then re-pack with latest webpack, my program throw a XXX is not defined error.

Here is a minimal code to reproduce the bug:

muzuiget:~/test$ cat src/lib.js 
export default {
    name: 'hello',
};
muzuiget:~/test$ cat src/index.js 
import lib from './lib';
class Hello {
    run1() {
        console.log(lib);
    }
    run2 = () => {
        console.log(lib);
    }
}
const hello = new Hello();
hello.run1(); // OK
hello.run2(); // ReferenceError: lib is not defined
$ node dist/main.js 
{ name: 'hello' }
/home/muzuiget/test/dist/main.js:1
(()=>{"use strict";const n={name:"hello"},l=new class{run1(){console.log(n)}run2=()=>{console.log(lib)}};l.run1(),l.run2()})();
                                                                                                  ^

ReferenceError: lib is not defined

@tobiu
Copy link

tobiu commented Apr 25, 2021

off topic: i would not recommend using public class fields to define methods.

fields get assigned inside the ctor on instance level and methods belong to the prototype (unless you want to duplicate them for each instance).

@muzuiget
Copy link

This usually use for callback, auto bind the this to current instance, this is recommend way on React https://reactjs.org/docs/handling-events.html

@NathanaelA
Copy link

@muzuiget - I'm going to say this is probably a bug in webpack, if you turn off optimization (optimization: {minimize: false}). in the webpack config you can see webpack thinks the two lib values are different variables. (Hence the output you get above, where one is rewritten properly and the other left as-is)

Interestingly enough if you use require instead of import, or const lib = {name: 'hello'}; and remove the import it parses everything correctly, only if you use a import statement does it think the lib value is different.

@alexander-akait
Copy link
Member

@NathanaelA Can you provide example? Yep, it may be bug

@NathanaelA
Copy link

Here is the demo I used from muzuiget -- @alexander-akait

Webpack.config.js (Same as first issue, but added disabling minimizing).

const { resolve } = require('path');

module.exports = {
    entry: './index.js',
    output: {
      path: resolve(__dirname, 'dist'),
      filename: '[name].js'
    },
    resolve: {
        extensions: [ '.js' ]
    },
    optimization: {minimize: false}
}

lib.js

export default {
    name: 'hello',
};

index.js

import lib from './lib';    // This one fails
// const lib = require("./lib"); // This works if un-commented

class Hello {
    run1() {
        console.log(lib);
    }
    run2 = () => {
        console.log(lib);
    }
}
const hello = new Hello();
hello.run1(); // OK
hello.run2(); // ReferenceError: lib is not defined

@sokra
Copy link
Member

sokra commented Apr 26, 2021

Since this reached stage 4 only recently and acorn added this only 2 days ago, we need also need to add this to webpack parser to be able to handle webpack constructs in the new syntax (e. g. imports)

@r002
Copy link

r002 commented May 11, 2021

I was previously using "webpack": "^5.24.2" and encountered this same problem this morning:

image

But as soon as I updated to "webpack": "^5.35.1", Webpack is happy again! 😁

image

Thank you for fixing this!!! 🙏🙏🙏

PS. As a side-note-- So basically, JS has become the "Everything Language" at this point? I honestly didn't even know that private class vars had come to JS until this morning... what a world! I've been recently working in Go (where there are no classes! Structs here! Structs there! Structs everywhere!) so returning to JS recently is kinda like returning to America... a place of radical freedom where everything/anything goes!! 😮

@alexander-akait
Copy link
Member

@r002 Can you update webpack? https://github.com/tc39/proposals/blob/master/finished-proposals.md Class Fields on stage 4 and acorn should parse them

@r002
Copy link

r002 commented May 11, 2021

@r002 Can you update webpack? https://github.com/tc39/proposals/blob/master/finished-proposals.md Class Fields on stage 4 and acorn should parse them

@alexander-akait Yes, I updated Webpack and that solved the problem! I was just confirming with you guys that everything's copacetic now-- thank you for updating Webpack to support the new spec! 🙏🙏🙏

@alexander-akait
Copy link
Member

I think we can close it, idea from #10216 (comment)

Allow to set plugins for acorn (first PR)
Resolved minimum supported node using browserslist/package.json and setup plugins by default (second PR)

Is not easy, the main problem here - handling new AST nodes, it requires many changes, including parser/plugins for optimizations/etc. Even we allow setup plugins it can still potentially break your code or behave unpredictably. So webpack supports only stage 4, when new syntax will be part of stage 4 webpack will support it.

Anyway feel free to open new issues when something will be in stage 4, and yes, we are watching this too.

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

No branches or pull requests