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

feat!(remix-react,remix-server-runtime): remove deprecated properties from HtmlLinkDescriptor, LinkDescriptor & PrefetchPageDescriptor types #5705

Conversation

MichaelDeBoey
Copy link
Member

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Mar 8, 2023

🦋 Changeset detected

Latest commit: d4269e0

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/react Major
@remix-run/server-runtime Major
@remix-run/testing Major
@remix-run/cloudflare Major
@remix-run/deno Major
@remix-run/dev Major
@remix-run/node Major
@remix-run/cloudflare-pages Major
@remix-run/cloudflare-workers Major
create-remix Major
@remix-run/css-bundle Major
@remix-run/architect Major
@remix-run/express Major
@remix-run/netlify Major
@remix-run/vercel Major
@remix-run/serve Major
remix Major
@remix-run/eslint-config Major

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

@MichaelDeBoey MichaelDeBoey added the BREAKING CHANGE This change will require a major version bump label Mar 8, 2023
@brophdawg11 brophdawg11 removed the BREAKING CHANGE This change will require a major version bump label Mar 8, 2023
@MichaelDeBoey MichaelDeBoey force-pushed the remove-HtmlLinkDescriptor-properties-in-v2 branch 2 times, most recently from b68b69e to 6b3bda6 Compare March 15, 2023 11:06
@MichaelDeBoey MichaelDeBoey force-pushed the remove-HtmlLinkDescriptor-properties-in-v2 branch 3 times, most recently from 82ad614 to f4bb542 Compare March 16, 2023 21:21
@chaance
Copy link
Collaborator

chaance commented Mar 16, 2023

Hey @MichaelDeBoey, I think we should actually keep these in place for now. We have no plans to drop support for React 17, and I don't want the console yelling at folks who are following older type defs. Gonna go ahead and close this for now!

@chaance chaance closed this Mar 16, 2023
@MichaelDeBoey
Copy link
Member Author

MichaelDeBoey commented Mar 16, 2023

@chaance This is not dropping React 17 support, it is only dropping the possible use of these props.

I still made sure that React 17 is supported, so I think we should re-open this PR

// In React 17, <link imageSrcSet> and <link imageSizes> will warn
// because the DOM attributes aren't recognized, so users need to pass
// them in all lowercase to forward the attributes to the node without a
// warning. Normalize so that either property can be used in Remix.
let imageSizesKey = "useId" in React ? "imageSizesKey" : "imagesizes";
let imageSrcSetKey = "useId" in React ? "imageSrcSet" : "imagesrcset";

[imageSizesKey]: imageSizes,
[imageSrcSetKey]: imageSrcSet,

@MichaelDeBoey MichaelDeBoey reopened this Mar 20, 2023
@MichaelDeBoey MichaelDeBoey force-pushed the remove-HtmlLinkDescriptor-properties-in-v2 branch from f4bb542 to f2ef5a8 Compare March 20, 2023 19:15
@brophdawg11
Copy link
Contributor

I added some filters for console.log assertions in #5706 prior to merging to avoid false negatives. Once dev is merged out to v2 again we should rebase this and remove those filters in this PR

@MichaelDeBoey MichaelDeBoey force-pushed the remove-HtmlLinkDescriptor-properties-in-v2 branch from f2ef5a8 to 4aabfed Compare March 23, 2023 17:03
@MichaelDeBoey
Copy link
Member Author

@brophdawg11 The extra changes you mentioned are now removed as well in this PR

@MichaelDeBoey MichaelDeBoey force-pushed the remove-HtmlLinkDescriptor-properties-in-v2 branch from 4aabfed to 9c01f4e Compare March 25, 2023 17:04
brophdawg11
brophdawg11 previously approved these changes Mar 27, 2023
@brophdawg11 brophdawg11 dismissed their stale review March 27, 2023 13:44

Oops - looks like a few integration tests are busted

@MichaelDeBoey MichaelDeBoey force-pushed the remove-HtmlLinkDescriptor-properties-in-v2 branch from 9c01f4e to 9c3536b Compare March 31, 2023 17:28
@MichaelDeBoey MichaelDeBoey force-pushed the remove-HtmlLinkDescriptor-properties-in-v2 branch from 9c3536b to b2bc277 Compare April 10, 2023 17:09
@MichaelDeBoey MichaelDeBoey force-pushed the remove-HtmlLinkDescriptor-properties-in-v2 branch from b2bc277 to 5fb82d9 Compare April 13, 2023 20:04
@brophdawg11 brophdawg11 self-assigned this Apr 13, 2023
@brophdawg11
Copy link
Contributor

@MichaelDeBoey Do you mind checking out the failed integration tests? Maybe they're using the syntax we're dropping support for in v2? Once those are fixed up I think we can get this merged 👍

…sizes` properties from `HtmlLinkDescriptor`, `LinkDescriptor` & `PrefetchPageDescriptor` types
@MichaelDeBoey MichaelDeBoey force-pushed the remove-HtmlLinkDescriptor-properties-in-v2 branch from 5fb82d9 to d4269e0 Compare April 14, 2023 16:36
@MichaelDeBoey
Copy link
Member Author

@brophdawg11 It was my own mistake, I used "imageSizesKey" as the attribute instead of "imageSizes" 😅🙈😓

Tests should pass now 👍

@brophdawg11 brophdawg11 merged commit bdd8e5e into remix-run:v2 Apr 14, 2023
9 checks passed
@MichaelDeBoey MichaelDeBoey deleted the remove-HtmlLinkDescriptor-properties-in-v2 branch April 14, 2023 18:45
MichaelDeBoey added a commit that referenced this pull request Jul 10, 2023
…escriptor` & `PrefetchPageDescriptor` types (#5705)
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.

None yet

3 participants