-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
feat: deprecate all functions & types exported from magicExports
#3284
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want @mjackson or @ryanflorence to approve this before merging.
0296fcf
to
9546a52
Compare
8c222f1
to
43c63ad
Compare
8a3d496
to
f30d803
Compare
@mjackson @ryanflorence Our ESLint config is warning with a deprecated message when importing from the |
f30d803
to
1b0e812
Compare
1b0e812
to
d956a2c
Compare
1446ff5
to
3f5d0ed
Compare
I think now is a great time to officially deprecate using magic exports 👍 Do you know why the tests are failing in the diff view @MichaelDeBoey? Is that something to do with this PR? Or just something that has been happening for a while now? I'd like to clean that up if we can. If the test failures are unrelated to this PR, it's a huge distraction. Can you also please say more about the approach you're taking here? It looks like you're generating a bunch of code in the build output. Is that right? I think I'd prefer to just write out the code (it's repetitive, I know) instead of introduce more complexity in our build. |
@mjackson If you're talking about the failing tests underneath
In #3543, @chaance removed all manual My changes are building upon that script, by wrapping all exported functions in a My changes also add an |
3f5d0ed
to
84779d8
Compare
@mjackson We moved to code generation here because having the code in the workspace was causing TypeScript issues when we did our Rollup build overhaul a while back. It was always intended to be temporary as we planned on removing the exports altogether, but we went this route to move a little quicker, as that refactor was blocking other work (specifically, adopting Changesets as noted in #3543). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These look good overall. Can you merge the latest from dev
one last time? Once that's taken care of I'll cut an experimental release to manually check the dev warnings, and if everything looks good we'll get this in for the next minor bump.
Thanks so much for seeing this through! 🙏
84779d8
to
4d9d062
Compare
e3f28ca
to
4caa205
Compare
@MichaelDeBoey Just FYI, the only reason I haven't merged this yet is because I want to time it with an upcoming minor release. Once @brophdawg11 and @jacob-ebey finalize their work integrating React Router 6.4 I'll merge this and ship them together. Just wanted you to know so you don't need to keep resolving conflicts until we're there! |
ca3f65d
to
8c35380
Compare
@MichaelDeBoey Ready to merge this if you're able to bring the branch up-to-date 😄 |
8c35380
to
d2e0c1f
Compare
@chaance Rebase onto latest |
* Revert "Deprecate all functions & types exported from `magicExports` (#3284)" This reverts commit 848d8b0. * Single deprecation warning per remix index file * Revert "Single deprecation warning per remix index file" This reverts commit 7774492. * Add back in TS deprecation typings * Warn on deprecated remix imports via esbuild plugin * add chgangeset
* Revert "Deprecate all functions & types exported from `magicExports` (#3284)" This reverts commit 848d8b0. * Single deprecation warning per remix index file * Revert "Single deprecation warning per remix index file" This reverts commit 7774492. * Add back in TS deprecation typings * Warn on deprecated remix imports via esbuild plugin * add chgangeset
I wanted to re-submit #2735 again, as:
1.3.3
, which was released on 23 Mar 2022https://github.com/remix-run/remix/releases/tag/v1.3.3
replace-remix-imports
migration was added in1.4.0
, which was released on 14 Apr 2022https://github.com/remix-run/remix/releases/tag/v1.4.0
remix
package, as they're not included in themagicExports
anymore (see both@remix-run/cloudflare
&@remix-run/node
) since1.5.0
@remix-run/deno
) that doesn't have any magic exports at all since1.5.0
https://github.com/remix-run/remix/releases/tag/v1.5.0
remix
package since1.6.0
https://github.com/remix-run/remix/releases/tag/v1.6.0
Original PR description: