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

Scripts: regression - fails to import fonts refered in imported css file from an external npm's dependency #66503

Closed
3 of 6 tasks
loxK opened this issue Oct 27, 2024 · 8 comments · Fixed by #66975
Closed
3 of 6 tasks
Assignees
Labels
[Status] In Progress Tracking issues with work in progress [Tool] WP Scripts /packages/scripts [Type] Bug An existing feature does not function as intended

Comments

@loxK
Copy link
Contributor

loxK commented Oct 27, 2024

Description

I have a block that uses flexslider. It fails to build since @wordpress/scripts version 29.0.0

Could it be related to that commit #61121 ?

Step-by-step reproduction instructions

  1. clone my test repo and install npm dependencies
mkdir test
cd test
git clone https://github.com/loxK/wordpress-scripts-test.git ./ -b font-dependency
npm i
  1. build : npm run build

  2. notice the errors importing the font referenced in the felxslider module :

ERROR in ./blocks/editor.scss (./blocks/editor.scss.webpack[javascript/auto]!=!./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[3].use[1]!./node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[3].use[2]!./node_modules/sass-loader/dist/cjs.js??ruleSet[1].rules[3].use[3]!./blocks/editor.scss) 5:36-89
Module not found: Error: Can't resolve 'fonts/flexslider-icon.eot' in '/home/www/user/Tests/wordpress-scripts-test/blocks'
  1. downgrade to wordpress/scripts v28.6.0, and build :
npm i -D @wordpress/[email protected]
npm build
  1. notice that there are no errors and taht fonts are in assets/blocks/fonts :
# ls assets/blocks/fonts 
flexslider-icon.0c4bb125.eot  flexslider-icon.b5aefbb7.woff  flexslider-icon.c6c9e9e5.ttf

Screenshots, screen recording, code snippet

A test repository to reproduce is available here : https://github.com/loxK/wordpress-scripts-test/tree/font-dependency

Environment info

  • Node v20.18.0
  • Ubuntu 24.04.1 LTS

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes

Please confirm which theme type you used for testing.

  • Block
  • Classic
  • Hybrid (e.g. classic with theme.json)
  • Not sure
@loxK loxK added the [Type] Bug An existing feature does not function as intended label Oct 27, 2024
@loxK loxK changed the title Scripts: regression - fails to import fonts refered in imported css file from external node dependency Scripts: regression - fails to import fonts refered in imported css file from an external npm's dependency Oct 27, 2024
@loxK
Copy link
Contributor Author

loxK commented Oct 27, 2024

I can confirm that, using @wordpress/scripts version 29.0.0 and later, if I revert 34c075b by removing the line require( 'postcss-import' ) from the webpack config at node_modules/@wordpress/scripts/config/webpack.config.js, it builds correctly.

Should I submit a pull request to revert it ?

@t-hamano t-hamano added the [Tool] WP Scripts /packages/scripts label Oct 27, 2024
@gziolo
Copy link
Member

gziolo commented Nov 1, 2024

It look like https://www.npmjs.com/package/postcss-import needs some additional configuration to keep working with fonts and svgs.

@gziolo
Copy link
Member

gziolo commented Nov 5, 2024

A test repository to reproduce is available here : https://github.com/loxK/wordpress-scripts-test/tree/font-dependency

This was very helpful. I was able to play with it locally, and I fully understand the problem. The styles that get imported live inside node_modules/flexislider/ and after the styles get inlined the path to the fonts becomes relative to the current project, which obviously doesn't have these assets. I would be in favor of reverting these changes as proposed in #66503 (comment).

@benoitchantre
Copy link
Contributor

Does it help to replace @import by @use?

@import is deprecated and will be removed in Dart Sass 3.
https://sass-lang.com/documentation/at-rules/import/

@loxK
Copy link
Contributor Author

loxK commented Nov 11, 2024

Does it help to replace @import by @use?

No

@gziolo
Copy link
Member

gziolo commented Nov 13, 2024

I opened #66975 that reverts changes that introduced the issue.

@gziolo
Copy link
Member

gziolo commented Nov 18, 2024

npm publishing with the new @wordpress/scripts version containing the big fix should finish in about 10 minutes.

@loxK
Copy link
Contributor Author

loxK commented Nov 19, 2024

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Tool] WP Scripts /packages/scripts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants