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

Update webpack-related deps, clean up some deps #885

Merged
merged 7 commits into from
Sep 12, 2023
Merged

Conversation

askoufis
Copy link
Contributor

@askoufis askoufis commented Sep 7, 2023

Not really sure why we had webpack-dev-server pinned. Also no idea how require-from-string got included in this PR from a while ago. It's currently unused, and AFAICT it never was used.

Also removed the assert dev dep and changed all imports to node:assert/strict in tests. There is one more usage of node assert in a fixture, but that can't be changed to node:assert because webpack doesn't know how to load the node: protocol. I think adding the NodeTargetPlugin may be the solution, but don't really want to investigate that right now.

  • Removed the webpack-cli dev dep from sku, as it's not used. It's only used in the tests private package.
  • Removed the traverse dep from sku as it was unused

@askoufis askoufis requested a review from a team as a code owner September 7, 2023 04:53
@changeset-bot
Copy link

changeset-bot bot commented Sep 7, 2023

🦋 Changeset detected

Latest commit: 1fa0085

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
sku Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mrm007
Copy link
Contributor

mrm007 commented Sep 7, 2023

because webpack doesn't know how to load the node: protocol

It's the Babel plugin that needs to be configured to recognise node:assert/strict. You can verify it works OOTB with node:assert.
Webpack shouldn't even see the import, because Babel runs before Webpack.

@askoufis
Copy link
Contributor Author

askoufis commented Sep 7, 2023

because webpack doesn't know how to load the node: protocol

It's the Babel plugin that needs to be configured to recognise node:assert/strict. You can verify it works OOTB with node:assert. Webpack shouldn't even see the import, because Babel runs before Webpack.

Babel unasssert is only run during production builds/prod dev server. So it doesn't work with regular sku start, which is the whole point.

@askoufis
Copy link
Contributor Author

Also realised that webpack-cli wasn't used in sku as a dev dep, so I removed it.

@askoufis
Copy link
Contributor Author

Found another unused dep, traverse.

@askoufis askoufis merged commit a65bcde into master Sep 12, 2023
4 checks passed
@askoufis askoufis deleted the update-deps branch September 12, 2023 00:15
@seek-oss-ci seek-oss-ci mentioned this pull request Sep 12, 2023
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.

None yet

2 participants