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

fix(serverBareModulesPlugin): update require.resolve to use full import, update warning #3656

Merged
merged 5 commits into from
Jul 11, 2022

Conversation

mcansh
Copy link
Collaborator

@mcansh mcansh commented Jul 5, 2022

Signed-off-by: Logan McAnsh logan@mcan.sh

Closes: #

  • Docs
  • Tests

Testing Strategy:

…rt, update warning

Signed-off-by: Logan McAnsh <logan@mcan.sh>
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thanks! I think we should probably have a test for this functionality (not just the warning, but that too). Could you implement that?

@mcansh
Copy link
Collaborator Author

mcansh commented Jul 6, 2022

Thanks! I think we should probably have a test for this functionality (not just the warning, but that too). Could you implement that?

i agree, in my initial PR i wasn't sure about the best way to test for that being that a missing dependency would fail the build in esbuild.build, but i'll look into how others are doing that detection (and how we are for esm warnings)

this test also covers installed modules as it uses @remix-run/node and @remix-run/react

Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
@changeset-bot
Copy link

changeset-bot bot commented Jul 11, 2022

🦋 Changeset detected

Latest commit: 09008c6

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

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

@mcansh mcansh requested a review from kentcdodds July 11, 2022 16:34
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Coolio 👍 Thanks!

@kentcdodds kentcdodds merged commit de9fc05 into dev Jul 11, 2022
@kentcdodds kentcdodds deleted the logan/update-missing-dep-check branch July 11, 2022 18:27
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-de9fc05-20220712 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

serverBareModulesPlugin displays incorrect warnings for subpath imports in 1.6.2
3 participants