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

Error when using material-design-iconic-font with new CLI/require #934

Closed
cluka opened this issue Oct 11, 2018 · 10 comments · Fixed by #938
Closed

Error when using material-design-iconic-font with new CLI/require #934

cluka opened this issue Oct 11, 2018 · 10 comments · Fixed by #938

Comments

@cluka
Copy link

cluka commented Oct 11, 2018

I'm submitting a bug report

  • Library Version:
    ^1.0.0-beta.1

Please tell us about your environment:

  • Operating System:
    Windows [10]

  • Node Version:
    10.10.0

  • NPM Version:
    6.4.1
  • Browser:
    all

  • Language:
    TypeScript 3.0.1

  • Loader/bundler:
    RequireJS

Current behavior:
au start displays an error when it tries to bundle material-design-iconic-font

  • What is the expected behavior?
    The application should start without error

Reproduce:

  1. npm install aurelia-cli -g
  2. au new
  3. ... selected custom bundler then selected RequireJS
  4. restored packages with yarn
  5. yarn add material-design-iconic-font
  6. Add
    "copyFiles":{ "node_modules/material-design-iconic-font/dist/fonts/*":"material-design-iconic-font/dist/fonts" }, into aurelia.json after "bundles"
  7. Add
    <require from="material-design-iconic-font/dist/css/material-design-iconic-font.min.css"></require>
    into app.html
  8. au run

Error is in the image
cli-error

  • What is the motivation / use case for changing the behavior?
@zewa666
Copy link
Member

zewa666 commented Oct 11, 2018

@huochunpeng the main of the package is an array. Never seen that

@cluka
Copy link
Author

cluka commented Oct 11, 2018

Ok.
So it's all ok with my registration of the font in Aurelia bundler?
And this font does work without problem in Aurelia webpack

@zewa666
Copy link
Member

zewa666 commented Oct 11, 2018

@cluka actually no idea, but thats what I'd guess. Just mentioned that I wasn't aware you could use an array for main, so perhaps this is causing the new aurelia auto-tracer to choke

@cluka
Copy link
Author

cluka commented Oct 11, 2018

Maybe, I see that there are multiple pull request on it's github that fixes the main that should be string and to move the array to files.

@3cp
Copy link
Member

3cp commented Oct 11, 2018

There are two issues here.

  1. strange main. They definitely didn't follow spec https://docs.npmjs.com/files/package.json#main
    This is not a bug for us to fix, it's one for them.

This issue can be fixed with an explicit config

"dependencies": [
  //...
  {
    "name": "material-design-iconic-font",
    "path": "../node_modules/material-design-iconic-font",
    "main": "dist/css/material-design-iconic-font.min.css"
  }
]

For that reason, this issue can be closed.

  1. I saw you updated your post to use <require from="some.css"></require>. cli+requirejs has never supported import "some.css" in js file, our cli+webpack supports it with style-loader plugin.

I think it could be a good improvement to cli+requirejs to support import "some.css", so people can easily migrate their code from webpack app to requirejs app. I will keep it on my todo list.

@3cp
Copy link
Member

3cp commented Oct 12, 2018

I will make cli more resilient in this situation, instead of panic, I can ignore the malformed main. You will see a missing main warning, but css import will still work.

@3cp
Copy link
Member

3cp commented Oct 12, 2018

The fix means you don't need explicit config in aurelia.json.

@cluka
Copy link
Author

cluka commented Oct 12, 2018

@huochunpeng Damn you're fast :-). Thanks man.
About the import edit, yes i did replace it with require, as it was like you thought, I moved from webpack.

I think it could be a good improvement to cli+requirejs to support import "some.css", so people can easily migrate their code from webpack app to requirejs app.

It's fine for me, now that i know how to import them.

@3cp
Copy link
Member

3cp commented Oct 12, 2018

Congratulations for migrating from webpack to cli bundler!

It will take some time to heal your trauma, you might still wake up in nightmare in next few weeks.
Deep breathe, brother, Deep breathe... it was all over now.
🤣

@cluka
Copy link
Author

cluka commented Oct 12, 2018 via email

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

Successfully merging a pull request may close this issue.

3 participants