Skip to content

Conversation

@mturley
Copy link
Collaborator

@mturley mturley commented Apr 28, 2022

I was going to also upgrade react-router to v6 in this PR, but we depend on react-router-last-location which doesn't yet support v6, so I'm holding off on that for now.

Breaking changes in major version upgrades:

  • copy-webpack-plugin v10

    • minimum supported Node.js version is 12.20.0
    • update globby to 12.0.2 version

    These are okay for us because we're supporting the LTS version of Node and we're not using globby directly anywhere.

  • webpack-dev-server v4

    Breaking changes are detailed in the migration guide. This PR applies the required changes. Notable config changes include:

    • Removed the compress: true option since it is true by default now
    • Moved the basePath to static.directory
    • Moved the overlay to client.overlay
    • Applied Prettier style to all webpack config files

@mturley mturley changed the title Upgrade webpack-dev-server to v4 and other webpack dependencies to latest Upgrade webpack-dev-server to v4 and other webpack dependencies to latest minor versions Apr 28, 2022
Copy link
Contributor

@seanforyou23 seanforyou23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice @mturley! I think I found a fix for an issue where non-svg bg images were not working.. let me know if it makes sense. I'm sure we'll rework how assets are loaded before long, but that should keep them in good working order until then.

Everything else looks good to me, nice work!

'node_modules/@patternfly/react-inline-edit-extension/node_modules/@patternfly/react-styles/css/assets/images'
),
],
use: [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran into an issue where using jpg and similar images as background images in css files wasn't working. Took me a while but I was able to hunt it down - looks like those new asset modules are applied by default, and since we're using the "old" asset loaders it was causing webpack to process these assets twice, and the resulting asset produced for images in stylesheets was corrupt.

If we add type: 'javascript/auto' to this loader definition we get a module output where the path should be, which we can then just add esModule: false, to the options to get the intended path back.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @seanforyou23 . Sorry to be coming back to this so late, and I know you're elsewhere now so it's cool if you can't or don't want to respond.

I'm not able to reproduce the issue you decribe here. With the existing webpack config, I can put a .png file in app/src/bgimages and use it as a background in my CSS and it works fine with or without the options you mentioned. I do have to do a full npm run build for the new images to be picked up, though.

I think I'm going to say that if there's still an issue here it may fall outside the scope of this PR? If you feel like following up I'm happy to open another issue and fix it. 🍻

@mturley mturley merged commit 8441bb6 into patternfly:main Aug 18, 2022
@mturley mturley deleted the upgrade-wds branch August 18, 2022 17:12
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 this pull request may close these issues.

2 participants