Skip to content

wont work with webpack2 #17

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

Closed
digitalkaoz opened this issue Feb 8, 2016 · 16 comments
Closed

wont work with webpack2 #17

digitalkaoz opened this issue Feb 8, 2016 · 16 comments

Comments

@digitalkaoz
Copy link

it seems this loader wont work with webpack2:

//webpack.config.js
        loaders: [{
            test: /\.js$/,
            loaders: ['ng-annotate', 'babel'],
            exclude: /node_modules/
        }]

using https://github.com/gajus/babel-preset-es2015-webpack (to enable tree-shaking)

//babel.rc
{
  "presets": ["es2015-webpack"],
  "plugins": [
    "transform-class-properties"
  ]
}
{
  "devDependencies": {
    "babel-core": "^6.2.1",
    "babel-loader": "^6.2.0",
    "babel-plugin-transform-class-properties": "^6.4.0",
    "ng-annotate-loader": "^0.1.0",
    "webpack": "^2.0-beta",
    "webpack-dev-server": "^2.0-beta"
  }
}
//package.json

none of my angular files gets injections anymore...any hint?

btw im using the "ngInject" syntax

@andrey-skl
Copy link
Owner

@digitalkaoz So I investigated the issue and found that not a webpack 2 itself but es2015-webpack preset. It doesn't convert imports and exports from ES6 to commonJS so ng-annotate couldn't understand this.

So you could still be possible to use standart es2015 preset instead and get CommonJS as output of babel loader. I believe this is not a big deal to miss webpack's ES6 processing feature, isn't it?

andrey-skl added a commit that referenced this issue Feb 8, 2016
@digitalkaoz
Copy link
Author

mh, webpack2 has a native import feature. i tried to avoid the common-js-module plugin so webpack2 could optimize the code (see http://www.2ality.com/2015/12/webpack-tree-shaking.html) . would be great if your loader can support this...

maybe it must be a post-loader?

@andrey-skl
Copy link
Owner

@digitalkaoz I would like it to support this native import too but ng-annotate-loader is just wrapper for olov/ng-annotate module which does not support es6 features.

So the only way is to convert js to valid es5 before passing to ng-annotate. post-loader will not help here as I know because post- means only that loader will be applied after just loaders.

@iboozyvoozy
Copy link

some news about webpack@2 + es2015-webpack + ng-annotate ?

@andrey-skl
Copy link
Owner

Unfortunately no because ng-annotate module will support this only if somebody contribute this. There is nobody who'd make this done at the moment.

@gaui
Copy link

gaui commented Jan 5, 2017

I didn't get it to work with Webpack 2 so I tried ng-annotate-webpack-plugin and it worked.

@crucialfelix
Copy link

I switched to "babel-plugin-angularjs-annotate" and I now have annotation working in webpack 2 with tree shaking.

This means using babel to do the annotation transform, not using any webpack loaders or plugins.

@colinskow
Copy link

babel-plugin-angularjs-annotate caused runtime errors in my app.

ng-annotate-webpack-plugin works perfectly with Webpack 2 tree shaking. This needs to be a plugin rather than a loader.

@nstuyvesant
Copy link

nstuyvesant commented Feb 26, 2017

Been using ng-annotate-loader with Webpack 2.2.1 successfully with this as an element in my array for config.module.rules:

{
  test: /\.js$/,
  loader: 'ng-annotate-loader?single_quotes',
  enforce: 'post'
}

Also tried ng-annotate-webpack-plugin which works but is much slower (could be my configuration though).

@Verdier
Copy link

Verdier commented Mar 27, 2017

You have to suffix by -loader => ng-annotate-loader and babel-loader.

    module: {
        rules: [{
            test: /\.js$/,
            exclude: /node_modules/,
            use: [{
                loader: 'ng-annotate-loader'
            }, {
                loader: 'babel-loader',
                options: {
                    presets: [
                        ["es2015", {"loose": true}]
                    ]
                }
            }]
        }]
    },

@bluetech
Copy link
Contributor

@huston007 What do you think about adding an option to specify how ngAnnotate is imported? Then it will be possible to use a fork of ng-annotate which accepts newer JS syntax. For my purposes, the following patch to ng-annotate suffices:

diff --git a/ng-annotate-main.js b/ng-annotate-main.js
index 78ca8df..41a701d 100644
--- a/ng-annotate-main.js
+++ b/ng-annotate-main.js
@@ -1075,8 +1075,9 @@ module.exports = function ngAnnotate(src, options) {
         stats.parser_parse_t0 = Date.now();
         // acorn
         ast = parser(src, {
-            ecmaVersion: 6,
-            allowReserved: true,
+            ecmaVersion: 8,
+            allowImportExportEverywhere: true,
+            allowReturnOutsideFunction: true,
             locations: true,
             ranges: true,
             onComment: comments,
diff --git a/package.json b/package.json
index f0b1673..0852387 100644
--- a/package.json
+++ b/package.json
@@ -8,7 +8,7 @@
     "url": "https://github.com/olov/ng-annotate.git"
   },
   "dependencies": {
-    "acorn": "~2.6.4",
+    "acorn": "~5.0.3",
     "alter": "~0.2.0",
     "convert-source-map": "~1.1.2",
     "optimist": "~0.6.1",

[for some reason ng-annotate has a test which rejects caret (^) dependencies, otherwise I would have used that.]

It should not be a problem to publish that to a new package, ng-annotate-next or something like that.

@andrey-skl
Copy link
Owner

@bluetech hm, actually this could work. Something like:

{
  test: /src.*\.js$/,
  use: [
    {
      loader: 'ng-annotate-loader',
      options: {
        add: false,
        map: false,
        ngAnnotatePath: path.resolve(__dirname, '..path/to')
      }
    }
  ]
}

@bluetech
Copy link
Contributor

I think one of these two methods will be better than having to pass a full path:

  1. Passing just the package name, e.g. "ng-annotate-next", then require() then inside the loader.
  2. Passing the actual ngAnnotate function, i.e. let the user do the require themselves.

I personally prefer 2, if it's possible with webpack, but 1 is also fine.

I think the option should be called just ngAnnotate, but that doesn't matter much.

bluetech added a commit to bluetech/ng-annotate-loader that referenced this issue Apr 28, 2017
ng-annotate is no longer maintained[1], and hence fails when applied to
source code containing modern JavaScript constructs, like import and
export. In particular, this prevents usage with Webpack's ES6 modules
support (issue andrey-skl#17).

[1] olov/ng-annotate#245

To work around it, allow the user to specify a fork of ng-annotate,
which adds support for new JS syntax.
@bluetech
Copy link
Contributor

OK, since version 0.6.0 of the loader it is now possible to specify a fork of ng-annotate. I had made such a fork which fixes the issues I had encountered around import/export. The fork is here: https://github.com/bluetech/ng-annotate-patched see the README for the changes.

If you want to use that, install the fork:

npm install --save-dev ng-annotate-patched

And tell the loader to use it:

{
    loader: 'ng-annotate-loader',
    options: {
        ngAnnotate: 'ng-annotate-patched',
    },
}

That should be it.

@andrey-skl
Copy link
Owner

So I assume this issue is closed that way. Thanks everybody for help.

@irshsheik
Copy link

babel-plugin-angularjs-annotate is the updated version of ng-annotate. it works by adding the plugin to babelrc file.
very convenient and easy to use

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

10 participants