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

Add more support in @wordpress/scripts to widen the usage of the package #27581

Closed
kimmenbert opened this issue Dec 8, 2020 · 3 comments · Fixed by #28043
Closed

Add more support in @wordpress/scripts to widen the usage of the package #27581

kimmenbert opened this issue Dec 8, 2020 · 3 comments · Fixed by #28043
Assignees
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] In Progress Tracking issues with work in progress [Tool] WP Scripts /packages/scripts [Type] Enhancement A suggestion for improvement.

Comments

@kimmenbert
Copy link

kimmenbert commented Dec 8, 2020

I think @wordpress/scripts could be the build tool to turn to when in need of build tool in WordPress theme / plugin development.

So far using it in theme, with both .js files and .scss files as entry points I have encountered these issues:

  • If you add a png/jpg etc image as background image in css, the build fails because missing rules for those kind of files.
  • If you include local font files in the css, the build fails because missing rules for those kind of files

Got this working by extending @wordpress/scripts webpack file to include support for those.

After support got added there was another issue caused by the CleanWebpackPlugin plugin. The images / fonts will be generated every build and on the initial wp-scripts start but while watching and recompiling the files is removed and not re-compiled.

My addition to the webpack config:

rules: [
  ...WordPressConfig.module.rules,
  {
    test: /\.(png|jp(e*)g|gif)$/,
    use: [
      {
        loader: 'file-loader',
        options: {
          name: 'images/[name].[ext]',
        },
      },
    ],
  },
  {
    test: /\.(woff|woff2|eot|ttf|otf)$/,
    use: [
      {
        loader: 'file-loader',
        options: {
          name: 'fonts/[name].[ext]', // TODO: if multiple folders inside the fonts folder with the same filename inside the folders. this will override files with the latest compiled file
        },
      },
    ],
  },
],
@gziolo gziolo added [Tool] WP Scripts /packages/scripts [Type] Enhancement A suggestion for improvement. labels Dec 8, 2020
@gziolo
Copy link
Member

gziolo commented Dec 8, 2020

The interesting part is that also today we have been discussing support for fonts in #27565 in a completely different issue. In the gist, in the tutorial there was a section that explained how to add support to webpack for fonts 😄

Furthermore, if the user ignores the mismatched files and continues on to complete the block using SCSS, when including the custom font's .otf file, webpack fails to compile the files correctly. The user has to extend the webpack.config.js file from @wordpress/scripts by creating their own file in the block and adding loading rules to compile the font. The process for making the SCSS method work is explained in the edits to the original tutorial suggested here. The edits show one method to extend the webpack config in order for the custom font to work.

@gziolo
Copy link
Member

gziolo commented Dec 8, 2020

Thank you for starting the discussion during the weekly WordPress Core JS chat. I'm personally open to adding more changes like that since it shouldn't impact the size of @wordpress/scripts since all those files use the same loader. At the same time, it might drastically improve the overall experience. In addition to that, create-react-app seems to do it as well.

Images

https://github.com/facebook/create-react-app/blob/39689239c18a1d77fb303e285b26beb1a4b650c0/packages/react-scripts/config/webpack.config.js#L388-L398

Fonts

I couldn't find a specific section but there is catch it all that might trigger in this situation:

https://github.com/facebook/create-react-app/blob/39689239c18a1d77fb303e285b26beb1a4b650c0/packages/react-scripts/config/webpack.config.js#L575-L590

@gziolo gziolo added the Needs Dev Ready for, and needs developer efforts label Dec 12, 2020
@gziolo gziolo added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Dec 12, 2020
@gziolo gziolo added the [Status] In Progress Tracking issues with work in progress label Jan 7, 2021
@gziolo gziolo self-assigned this Jan 7, 2021
@gziolo gziolo removed the Needs Dev Ready for, and needs developer efforts label Jan 7, 2021
@gziolo
Copy link
Member

gziolo commented Jan 7, 2021

@kimmenbert, I have a working proposal in #28043. I found a way to work around this annoying issue with CleanWebpackPlugin you mentioned. It removes unused assets (images and fonts) only on build and initial start run.

gziolo added a commit that referenced this issue Jan 9, 2021
* Scripts: Add supports for static assets in build commands
Fixes #27581.

* Docs: Add details how to use fonts and images

* Use file loader for images
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] In Progress Tracking issues with work in progress [Tool] WP Scripts /packages/scripts [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants