Module parse failed

[kmalakoff]

I am using webpack and babel to package this library, but get the following error:

ERROR in ./~/clipboard/src/clipboard.js
Module parse failed: 
/node_modules/clipboard/src/clipboard.js Line 1: Unexpected token
You may need an appropriate loader to handle this file type.
| import ClipboardAction from './clipboard-action';
| import Delegate from 'delegate-events';
| import Emitter from 'tiny-emitter';

I see you have configured the browser version to point to a different file than in Node.js so I am manually pointing to clipboard/dist/clipboard.js to work around this.

If I copy clipboard.js and clipboard-action.js into my project, they seem to work so I think it has something to do with the way package.json is set up.

[zenorocha]

Hey @kmalakoff,

Thanks a lot for letting us know, could you check that @mauriciosoares?

[mauriciosoares]

Hey @zenorocha, I don't think I'll be able to focus so much in this project, If I have some free time I'll try to check it, but feel free to ask someone else to help you out with it.

:)

[cironunes]

@kmalakoff what is your babel modules configuration?

[nuxlli]

@kmalakoff I just had to change babel loader at webpack.config.js:

//...
  module: {
    loaders: [
      {
        test: /\.js$/,
        include: [
          path.resolve(__dirname, "src"),
          path.resolve(__dirname, "node_modules/clipboard/src")
        ],
        loader: 'babel'
      }
    ]
  },
//...

[kmalakoff]

This is my configuration:

{test: /\.js?$/, exclude: /(node_modules)/, loader: 'babel?optional[]=runtime&stage=0'},

This is a general purpose webpack config so I would prefer not to add those include options specifically for this library. Any idea why those lines are necessary? Also, why does the browser needs to require the src directory instead of dist?

I can include the compiled code in dist just fine although I do get a warning This seems to be a pre-built javascript file. I think this might be because you are using browserify instead of webpack to release the built code. If you want an example of webpack for packaging libraries, you can check out one I wrote here.

[vccortez]

@zenorocha could this be solved by changing the browser field in package.json to dist/clipboard.js?

[daviferreira]

Just ran into this. :)

@kmalakoff The problem is clipboard modules use ES6, and you are excluding node_modulefrom your loader, so it's not being compiled.

One normal solution is to provide a dist-modules directory, with babel compiled modules. So they would still be modules, but in "regular" javascript. But if it doesn't make sense to use the modules separately, I would just suggest pointing to the dist files on package.json.

[thecotne]

@kmalakoff as @daviferreira mentioned you just need to remove exclude: /(node_modules)/

[kmalakoff]

Hmmmm. I definitely do not want to compile node_modules since modules should be distributed in an immediately useable form and it will be a productivity hit to compile all of the others when they are not necessary.

What I do on my compiled-to projects is use webpack to generate a compiled, dist version in universal module (UMD) form with all of the dependencies externally provided. For example, take a look at react-nvd3.

Would that work for clipboard.js? eg. distribute in a UMD form with external symbols

[natorojr]

@vekat's has a valid point. The "browser" field in your package.json should point to "dist/clipboard.js", not "src/clipboard.js", since dist/ is where you're outputting the Browserify build which you reference in the documentation.

When I change the "browser" field locally in node_modules/clipboard/package.json, the ERROR goes away and Webpack simply gives a WARNING instead

This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results.
@ ./~/clipboard/dist/clipboard.js"

Which is better than an error.

[kmalakoff]

@zenorocha regarding the help wanted label, I submitted a fix a while ago. The pull request just needs to be accepted.

[zenorocha]

Don't worry @kmalakoff. I'm just super busy right now but it's on my todo list

[craig-jennings]

Webpack allows you to easily alias the destination of clipboard to use an already compiled version of it. I have this in my webpack config:

resolve: {
  alias: {
    clipboard: 'clipboard/dist/clipboard',
  }
}

You'll get the same warning that @natorojr mentions, but it removes the need to compile clipboard via the babel-loader.

[kmalakoff]

@craigjennings11 the library should be fixed so workarounds are unnecessary. See: #85

[craig-jennings]

@kmalakoff If that's the standard, then you have a lot of libraries to update 😉

[kmalakoff]

@craigjennings11 totally, but it is worth it.

Standards for library authors really help the users of their libraries...imagine how many people run into the same problem and the amount of wasted effort there is finding workarounds. We've all got better things to do!

[zenorocha]

I have the same feeling as @craigjennings11. This is a library written in ES6 and the way you transpile ES6 in Webpack is by using babel-loader.

[kmalakoff]

@zenorocha if you believe that changing the way webpack works is best way to resolve this, please file a feature request or bug report with webpack then and link to it here.

Of course, if I look at my applications and clipboard is the only one that requires custom webpack configuration (as it is), I'd venture to guess that your feeling is probably not in the majority.

Either way, as a library author, you should make sure the users of your library do not waste time on non-value added activities so please let's get this resolved. I've volunteered to submit a solution to standardize clipboard's client distribution through pre-compilied, UMD-packaged assets which could be a good way to handle this in the short term.

[zenorocha]

I don't need to change webpack because it already offers what you need to import Clipboard.js.

Here are two different ways of using Clipboard.js with Webpack:

• Compiling Clipboard.js' source using Webpack as suggested by @nuxlli
• Bundling Clipboard.js' distribution with Webpack as suggested by @craigjennings11

[kmalakoff]

@zenorocha sorry, but those are workarounds, not solutions. A properly distributed library should require zero custom webpack configuration.

If you do not agree with this premise as a library author, then there is no reason for me to continue trying to help. 😉

[kmalakoff]

The pull request to fix this has been closed: #85.

Closing this issue.

[zenorocha]

I do want to offer the best experience for everybody when requiring this library. But your proposed solution included a bunch of opinionated tooling and your own personal preferences.

I'll leave this issue open for now, if anybody has an idea on how to improve it.

[kmalakoff]

No that's fine. Let's keep it closed since your main line of argument is that no changes are needed since all of your library users can make unnecessary changes to their webpack configuration files to support clipboard. I'm not in agreement with this as a principle of open source library distribution.

As for the PR, I've explained my time availability for volunteer work and responded to your feedback in the PR. You can merge and update to your alternative beliefs easily enough, but have instead decided to be a bit confrontational This PR is filled with personal preferences and opinionated tooling that are not truly necessary. (what tooling and programming isn't!) and to reject the goodwill of my volunteer work (mainly on a basis of bike-shedding arguments) rather than quickly modifying it to triage the issue.

[zenorocha]

No problem, moving discussion about main and browser fields to #105 /cc @vekat @natorojr

[heldr]

4 - Considerate of all users, not just the contributor
It’s easy to look at an improvement to a piece of code that fits your specific needs, but it may not be the best improvement for the majority of the users.

I often get patches and pull requests for ack where the patch makes sense for the author, but doesn’t for the community of users as a whole. While you don’t necessarily need to understand the project’s entire user base, sometimes your change might be too focused and specific for inclusion in the main project. In times like that, see if there’s an alternate way to get your change in the project. For example, maybe a behavior that doesn’t make sense to change for the entire project could be implemented with an alternate command line switch or configuration parameter.

https://blog.newrelic.com/2014/11/05/open-source-contribution/

[kmalakoff]

@heldr no problem with having my PR closed or rewritten. Totally agreed that the library maintainer / author needs to ensure solutions are best for all users. That's not what this issue or the PR is really about...

Like other users, I just want this problem solved. I do not agree with the premise that the library users should modify their configurations as @zenorocha was arguing and users shouldn't have to build the library using babel for the browser. Unnecessary configuration simply wastes people's time when it can actually be fixed / removed pretty easily (which is why I took the time to submit the PR in the first place...a little work on my side could help new users save time bumping into configuration or build problems).

To respond quickly to this issue, the contributors on clipboard could quickly modify my PR to triage the issue (removing any irreversible library decisions like embedding references to embedded modules, etc and putting preference-based bike-shedding issues aside) and then buy the time to solve it with the tooling they prefer.

At least this thread is creating the impetus for action!

[zenorocha]

Your intentions were good, but your pull request not, period.

The way you presented, this was a problem specific to Webpack users which wasn't really the case. It affected all module loaders because people were requiring a ES6 module from npm that wasn't transpiled.

This problem is now fixed in v1.5.3. No need for drama.