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

Cannot use within webpack #20

Closed
mikelambert opened this issue Apr 26, 2017 · 11 comments · Fixed by #22
Closed

Cannot use within webpack #20

mikelambert opened this issue Apr 26, 2017 · 11 comments · Fixed by #22
Assignees

Comments

@mikelambert
Copy link

I'm using webpack to compile some serverside javascript. One of these files builds an mjml server that transitively depends on datauri. Webpack gives me the following error:

Module parse failed: .../node_modules/datauri/index.js Unexpected character '#' (1:0)
You may need an appropriate loader to handle this file type.
SyntaxError: Unexpected character '#' (1:0)

I can fix this in my webpack config, by writing a custom loader that takes the file and then deletes any beginning-#-lines. But note that of all the npm modules I'm using, this is the only one. I'll probably end up doing this to clean up this webpack error, and avoid blocking on this Issue.

From what I've seen in other threads, it's considered "bad practice" (according to The Internetz) to have a file act as both a script as well as a library module, for this reason.

I'm filing a bug, to track fixing this, or to close out my Issue as Working As Intended/Will Not Fix. :)

@mikelambert
Copy link
Author

FWIW, seems there is a shebang-loader that will solve this problem for me within webpack. So the fact that I don't need to write my own loader/code (and others have encountered it with a few different npm modules out there), makes this feel like less of a datauri issue.

But I know there are numerous webpack issues about this, where they keep claiming it's the problem of the module authors, who are making libraries-that-are-also-scripts. (Since technically, # is technically not a valid javascript comment, and these are malformed JS files to include/require in a build setting.)

@john-osullivan
Copy link

john-osullivan commented Dec 6, 2017

@mikelambert I'm in the same position you are, trying to get mjml up and running. How did you bring shebang-loader into your webpack config? Based on the usage, it looks like the loader must be explicitly referenced in the require. Does that mean I need to fork mjml and fix any requires to datauri? Or were you able to simply preface the require to mjml?

Update for anyone else in my shoes: This shebang-loader gets the job done, there's a closed issue on its repo which shows how to properly set the webpack config: JavascriptIsMagic/shebang-loader#2 . Note to Windows users like myself, that issue recommends that we use path.resolve() to match on the offending npm package. That didn't work for me because you need to accurately specify the whole path. It was easier to use a RegEx that accounted for the different line break characters: /node_modules(\/|\\)datauri(\/|\\)index\.js$/

@mikelambert
Copy link
Author

Ah sorry for the delay in responding, just came to respond and saw you figured it out.

Just a heads-up, I was unable to get <mjml-style inline="inline"> working. I forget exactly, but that seems to pull in additional modules (as part of a css selector package, I think?), some of which do not work in a webpack environment since they do require(env.NODENEV === 'production' ? 'prod.js' : 'dev.js'). And even with a webpack.DEFINE_PLUGIN, webpack was having trouble when attempting to statically analyze that dependency.

Finally just gave up and am currently inlining my CSS in the tags where it's necessary (mainly to override link colors, which didn't work with a <style> tag due to some sort of precedence issues.)

@heldr
Copy link
Member

heldr commented Dec 11, 2017

I'm sorry guys, I was very busy with work this year. Gonna check webpack, rollup pretty sonner.

@heldr heldr mentioned this issue Dec 11, 2017
@WouterVanherck
Copy link

WouterVanherck commented Mar 9, 2018

@john-osullivan @mikelambert How exactly did you resolve it?
I tried to do it both ways but none ended up working:

module.exports = {
  module: {
    ...
    loaders: [
       ...
       {
         test: /node_modules(\/|\\)npm(\/|\\)bin(\/|\\)npm-cli\.js$/,
         loaders: ['shebang', 'babel']
       }
     ]
  }
  ...
};

and

module.exports = {
  module: {
    ...
    rules: [
        {
          test: path.resolve(String(path.appSrc), 'node_modules/npm/bin/npm-cli.js'),
          use: 'shebang-loader',
        }
      ]
  }
  ...
};

I've included these in my webpack.config.prod.js.
My #! was located in npm-cli.js.
O'm in "[email protected]"

@mir3z
Copy link

mir3z commented Jul 21, 2018

What is the status of this issue? It's more that one year old!

@ibc
Copy link

ibc commented Feb 20, 2019

I'm sorry guys, I was very busy with work this year. Gonna check webpack, rollup pretty sonner.

Not true.

@heldr
Copy link
Member

heldr commented Feb 21, 2019

Just a recall that PRs are always welcomed in open-source community and behind the scenes we have humans writing codes. Apparently, shamming contributors sounds an easier path though 😄

But yes! I should have prioritized it somehow, and sorry for those waiting for.

Being very transparent, I totally forget about this issue(can I?) and sometime I'll fix/review it.

Cheers! 🍵

@heldr heldr self-assigned this Mar 5, 2019
@heldr heldr closed this as completed in #22 Apr 17, 2019
@heldr heldr reopened this Apr 17, 2019
@heldr
Copy link
Member

heldr commented Apr 17, 2019

v2.0 FINALLY released https://github.com/data-uri/datauri/releases/tag/v2.0.0

Could any of you please recheck if it fixes webpack compatibility?

@DeedleFake
Copy link

I just checked. I can confirm that it works. Thank you.

@heldr
Copy link
Member

heldr commented Apr 18, 2019

Thank you all for reporting this and sorry for my absence last year. 🍵

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

Successfully merging a pull request may close this issue.

7 participants