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(dev): remove outdated esm import warnings #6916

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

pcattori
Copy link
Contributor

@pcattori pcattori commented Jul 21, 2023

Supercedes #6822

@changeset-bot
Copy link

changeset-bot bot commented Jul 21, 2023

🦋 Changeset detected

Latest commit: 9bcb0d9

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

This PR includes changesets to release 18 packages
Name Type
@remix-run/dev Patch
create-remix Patch
remix Patch
@remix-run/architect Patch
@remix-run/cloudflare Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
@remix-run/css-bundle Patch
@remix-run/deno Patch
@remix-run/eslint-config Patch
@remix-run/express Patch
@remix-run/netlify Patch
@remix-run/node Patch
@remix-run/react Patch
@remix-run/serve Patch
@remix-run/server-runtime Patch
@remix-run/testing Patch
@remix-run/vercel 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

@pcattori pcattori merged commit 22364f4 into dev Jul 21, 2023
9 checks passed
@pcattori pcattori deleted the pedro/remove-esm-warnings branch July 21, 2023 16:57
@github-actions github-actions bot added the awaiting release This issue has been fixed and will be released soon label Jul 21, 2023
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-974954f-20230722 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!

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-3b808ce-20230802 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!

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

🤖 Hello there,

We just published version 1.19.2-pre.1 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!

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-7cb1e7e-20230803 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!

@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2023

🤖 Hello there,

We just published version 1.19.2-pre.2 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!

@kentcdodds
Copy link
Member

Did this PR not make it into 1.19.2? I'm still seeing warnings with 1.19.2.

@pcattori
Copy link
Contributor Author

pcattori commented Aug 4, 2023

@kentcdodds should have landed in 1.19.1 or 1.19.2

We removed Remix-specified warnings that produced false positives, but you might still get actual warnings from esbuild. What do the warnings you are seeing look like?

@ngbrown
Copy link
Contributor

ngbrown commented Aug 4, 2023

This change didn't land in 1.19.1 or 1.19.2. I'm not sure why the bot thought it did.

The warnings shown are still the same:

> npx remix build

 info  building... (NODE_ENV=production)
 warn  esm-only package: quick-lru
┃ quick-lru is possibly an ESM-only package.
┃ To bundle it with your server, include it in `serverDependenciesToBundle`
┃ -> https://remix.run/docs/en/main/file-conventions/remix-config#serverdependenciestobundle
┗
 info  built (3.2s)
> npx remix --version
1.19.2

@kentcdodds
Copy link
Member

Yeah, I looked in node_modules and the code that was removed in this PR is definitely still there. 🤷‍♂️

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-6e250f7-20230805 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!

@kentcdodds
Copy link
Member

I just upgraded to 1.19.3 and this is still in there: https://unpkg.com/browse/@remix-run/dev@1.19.3/dist/compiler/server/plugins/bareImports.js (line 137)

@pcattori
Copy link
Contributor Author

pcattori commented Aug 9, 2023

Looks like this landed in dev branch after we switched dev to be our v2 branch. All releases until v2 have been hotfixes in main, so this will land as part of v2.

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-b1149bb-20230810 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
Labels
awaiting release This issue has been fixed and will be released soon CLA Signed package:dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants