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

[v1.0.0-rc] Remove deprecated features #2896

Merged

Conversation

callingmedic911
Copy link
Member

@callingmedic911 callingmedic911 commented Jun 26, 2021

Closes #2631.


Before merging, maybe we want recheck:

  • If we have any more deprecated features to remove.
    EDIT 29th Sept 21:
    • beforeResolver is deprecated in v0.37. Already removed
  • If we need anything from 15fdd40. Applied again.
  • Update docs Not required anymore.

This removes deprecated features planned for v1-rc.


Original:
I removed everything that says deprecated, even if it was marked deprecated only a few weeks ago. For example: 56f6019 (Supabase data field)
Also, I made sure one commits maps to individual features, so it's a bit easier to review commit-wise and revert.

The only deprecated annotation left is - processPagesDir(), which we can remove once we have the suggested alternate implementation.

@dac09 @jtoar @thedavidprice

@callingmedic911 callingmedic911 marked this pull request as draft June 26, 2021 08:13
@callingmedic911 callingmedic911 force-pushed the remove-deprecated-features branch from a3c1eac to ff10aee Compare June 26, 2021 08:20
@callingmedic911 callingmedic911 marked this pull request as ready for review June 26, 2021 08:42
@thedavidprice
Copy link
Contributor

Thank for getting this going @callingmedic911

@dac09 @peterp When do you think the timing is right for this one? Maybe v0.36, which will be within a couple of releases of v1-rc?

@thedavidprice
Copy link
Contributor

thedavidprice commented Jul 20, 2021

Hi @callingmedic911 Thank you for your patience with us on this one. I had a discussion with the core team maintainers, and we would like to handle this is two parts:

  1. deprecate some non-critical features now or in any other releases leading up to v1.0.0-rc.0
  2. wait for "critical" deprecations until the release of v1.0.0-rc.0

Our thinking it that we don't want to create much resistance to people upgrading now.

To Deprecate Now

For deprecations to include in our next release, I suggest:

  1. fully deprecating the obsolete commands, so any files in packages/cli/src/commands/* included in this PR. Note: we'll need to update the CLI documentation as well.
  2. the Forms package deprecations packages/core/src/index.ts and packages/forms/src/index.tsx

To deprecate with v1.0.0-rc.0

Then the remaining deprecation should happen along with the v1-rc release (along with anything else new that's scheduled for deprecation).


What do you think of this plan? If it makes sense, just let me know how you want to split the PRs. Maybe just create a new PR for the commands deprecation and then pull the changes into this PR? Loop me in however needed.

@thedavidprice thedavidprice changed the title Remove deprecated features [v1.0.0-rc] Remove deprecated features Jul 20, 2021
@thedavidprice thedavidprice added this to the future-release milestone Jul 20, 2021
@LBrian
Copy link
Contributor

LBrian commented Jul 21, 2021

@thedavidprice I had a quick look of this PR, agreed and makes sense we should cherry-pick form changes into #3043 for sure, I will do that later today. @callingmedic911 I will keep you in the loop.

  • it feels more appropriate to make those changes in this PR upgrading React Hook Forms [Error while deploying web side with prerender through Serverles [update fixed!]

Btw, just a kind reminding you might accidentally pasted the wrong PR number, just pointed out in case others get confused. :)

@thedavidprice
Copy link
Contributor

@LBrian gah, fixed! (thank you... numbers are hard sometimes.)

@LBrian LBrian mentioned this pull request Jul 21, 2021
@callingmedic911
Copy link
Member Author

callingmedic911 commented Jul 21, 2021

Our thinking it that we don't want to create much resistance to people upgrading now.

Totally agree.

  1. fully deprecating the obsolete commands, so any files in packages/cli/src/commands/* included in this PR. Note: we'll need to update the CLI documentation as well.

I guess this also includes api-server CLI command? I'll create a separate PR and cherry-pick commands deprecation.

  1. the Forms package deprecations packages/core/src/index.ts and packages/forms/src/index.tsx

I see @LBrian has cherry-picked 502159a and ff10aee. Thanks a lot @LBrian. :) I'll remove these commit from this PR now.
By the way, @thedavidprice also mentioned packages/core/src/index.ts, do we want to cherry-pick fd9a590? I don't think it's related to forms.

Then the remaining deprecation should happen along with the v1-rc release (along with anything else new that's scheduled for deprecation).

What do you think of this plan? If it makes sense, just let me know how you want to split the PRs. Maybe just create a new PR for the commands deprecation and then pull the changes into this PR? Loop me in however needed.

Sounds good! I'll use new commands deprecation branch as head for this one and we can merge remaining changes in v1-rc.

@thedavidprice
Copy link
Contributor

I guess this also includes api-server CLI command?

Ah, I forgot to mention that we should skip anything api-server related for now. Peter has changes coming, so let's wait for those.

By the way, @thedavidprice also mentioned packages/core/src/index.ts, do we want to cherry-pick fd9a590? I don't think it's related to forms

My mistake. I didn't mean to include that file. Let's skip for now 🤦‍♂️

Thanks!

@callingmedic911
Copy link
Member Author

Got it. Thank you!

@callingmedic911 callingmedic911 force-pushed the remove-deprecated-features branch from 8cafbf5 to f9b4738 Compare July 21, 2021 07:09
@callingmedic911 callingmedic911 force-pushed the remove-deprecated-features branch from f9b4738 to 3a1ed4f Compare July 21, 2021 07:28
Copy link
Contributor

@thedavidprice thedavidprice left a comment

Choose a reason for hiding this comment

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

Looping in others for specific questions.

Redwood-tools

I think it's time to fully deprecate redwood-tools.js

  • packages/cli/src/redwood-tools.js
  • references to redwood-tools and rwt in packages/cli
    • including references in packages/cli/README.md

Agreed @dac09?

__REDWOOD__API_PROXY_PATH

Looks like we still have a reference to remove over here:

  • packages/web/src/global.web-auto-imports.ts

Supabase.ts

@dthyresson do you have suggestions about how/if we should handle deprecations in:

  • packages/auth/src/authClients/supabase.ts

?

Scaffold.js

@cannikin any reason not to fully deprecate this in v0.39?

/** @deprecated Used to be able to create dbAuth pages with `yarn rw g scaffold dbAuth` */
if (modelArg.toLowerCase() === 'dbauth') {
console.info(c.green('\nGenerate dbAuth pages with:\n'))
console.info(' yarn rw generate dbAuth\n')
process.exit(0)
}

processPagesDir

@peterp @dac09 Is this still an intended deprecation?

/**
* Process the pages directory and return information useful for automated imports.
*
* Note: glob.sync returns posix style paths on Windows machines
* @deprecated I will write a seperate method that use `getFiles` instead. This
* is used by structure, babel auto-importer and the eslint plugin.
*/
export const processPagesDir = (
webPagesDir: string = getPaths().web.pages
): Array<PagesDependency> => {

@cannikin
Copy link
Member

@cannikin any reason not to fully deprecate this in v0.39?

Nope, go ahead and remove! I updated the docs recently to reference the new command yarn rw g dbAuth so we should be safe to remove.

@callingmedic911
Copy link
Member Author

Thanks for reviewing thoroughly @thedavidprice!

__REDWOOD__API_PROXY_PATH: Looks like we still have a reference to remove over here:

  • packages/web/src/global.web-auto-imports.ts

I'm not able to find any references to __REDWOOD__API_PROXY_PATH, am I missing something here?

Apart from this, I have removed dbAuth deprecation warning in the scaffold. Waiting for confirmation from others, then if required will those as well.

@dthyresson
Copy link
Contributor

@thedavidprice for

Supabase.ts

@dthyresson do you have suggestions about how/if we should handle deprecations in:

packages/auth/src/authClients/supabase.ts

I assume this question refers to:

  }): Promise<{
    user: User | null
    session: Session | null
    error: Error | null
    data: Session | User | null // Deprecated
  }>

to data.

I'll check as that may no longer be in the current Supabase js sdk.

@dthyresson
Copy link
Contributor

FYI for Supabase, can upgrade to latest and then remove deprecations, but relies on a fix: supabase/auth-js#178

@dthyresson
Copy link
Contributor

FYI for Supabase, can upgrade to latest and then remove deprecations, but relies on a fix: supabase/gotrue-js#178

supabase/auth-js#179 has been merged, just need to await the next supabase-js release which I believe well be next week.

@jtoar
Copy link
Contributor

jtoar commented Nov 19, 2021

@thedavidprice processPagesDir looks like something we can remove at any time in the RC process since it's internal. It's used in the structure package and a babel plugin (for auto-loading the routes in Routes.js) so it we also can't remove it without an alternative.

@thedavidprice
Copy link
Contributor

Finalizing and merging this now.

Moved Supabase deprecation step to #3756

Release Notes: Breaking

Router Globals

Fully deprecates:

  • __REDWOOD__API_URL
  • __REDWOOD__API_GRAPHQL_SERVER_PATH
  • __REDWOOD__API_PROXY_PATH

Use the following instead:

  • RWJS_API_GRAPHQL_URL
  • RWJS_API_DBAUTH_URL
  • RWJS_API_URL

@thedavidprice thedavidprice added the release:breaking This PR is a breaking change label Nov 19, 2021
@thedavidprice thedavidprice merged commit 44082ed into redwoodjs:main Nov 19, 2021
@callingmedic911 callingmedic911 deleted the remove-deprecated-features branch November 20, 2021 10:11
@thedavidprice thedavidprice modified the milestones: next-release, v0.39.0 Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:breaking This PR is a breaking change
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Remove deprecated features for v1
8 participants