fix(generator): Page generators accept default options too #2413

dac09 · 2021-04-30T16:54:22Z

What is it?

Fixes typescript generator for pages.

github-actions · 2021-04-30T16:57:58Z

📦 PR Packages

Click to Show Package Download Links

Install this PR by running yarn rw upgrade --pr 2413:0.31.0-8172fed

dthyresson

Had one question about storybook - but also what is the best way to test?

Pull the branch and babel node CRWA?

packages/cli/src/commands/generate/page/page.js

dthyresson · 2021-04-30T20:11:11Z

Oh @dac09 I meant to ask if tests and stories were generated by default now -- not just optional (but perhaps that's the PR's intent, too?)

dac09 · 2021-05-03T16:03:34Z

Oh @dac09 I meant to ask if tests and stories were generated by default now -- not just optional (but perhaps that's the PR's intent, too?)

I haven't really changed that behaviour here - they are default!

dac09 · 2021-05-03T16:26:44Z

Had one question about storybook - but also what is the best way to test?

rwt link is your friend!! If you have issues linking run yarn policies set-version 1.18.0 :)

dthyresson · 2021-05-03T18:11:29Z

@dac09 approved to merge

* fix(generator): Page generators accept default options too * Page generator also takes path positional * Fix command string with positionals

* feat(webhooks): Handle base64 encoded webhook payloads (#2340) * Handle base64 encoded payloads * Removes console logging. * Updates to Redwood-Render integration (#2336) * updating build and start abstractions * add boxen and improve setup deploy message * improve render.yaml format * removing region from static site * Handle focus on route change (#2321) * feat: focus RouteFocus on route change * feat: handle elements that aren't focusable * feat: add some tests for getFocus * style: split up tests * Fix "yarn rw test web" in e2e tests (#2329) * Copy rwjs packages to different location. * Run web tests. * Serve web/dist with `yarn rw serve web` (#2234) * Fix race condition in rwt link * Add new run command for api server * Update description for command * Rename run -> serve | Actually proxy the handler * Update packages/cli/src/commands/serve.js Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com> * PR Suggestions * Fix link * feat(rw-serv): Now supports serving web and both web+api * Regenerate yarn.lock from root * Refactor api-server file structure | Tests WIP * Update tests * Update packages/api-server/src/handler.ts Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com> * Read index content once, to prevent accessing FS every time Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com> * Verify signatures for secure webhooks and functions (#1843) * secureHandler verifies webhook signatures * Do not run verify as cannot acces env * Tests verify event. PR review updates. * WebhookVerificationError message is a string * Fixes Expected { after 'if' condition curly * Refactor secureHandler for a verifier * Get to green on ecureHandler.test * WIP verifier refactoring * Refactor into verifiers with types * Tests and implement verifyWebhook * Refactors signature header into options * Adds tests for none, secretKey, sha256 * Correct jsdocs * Adds ability to override event body payload * Refactor to use receiveAndVerify. Add tests. Support object payload for sha. * Add JWT verifier/Renames *Verifier & None to Skip * jwtVerifier wip * WIP * Add webhooks to api * Reorganize and add sha1 verifier * Adds more webhooks tests per verifier * Refactor createVerifier * Uses jwt.verify to check the issuer * Refactor circular dependency w separate types file * redwoodjs/api needs webhooks * Made VerifyOptions optional * Refactor WebhookVerifier interface w/ Danny * Cleanup jsdocs * Explains WEBHOOK_SECRET * Fix page generator taking in ts | Fix rw serve crashing out cli (#2335) * Fix page generator taking in ts | Fix rw serve crashing out cli * Change e2e to pass --typescript param * Remove unneeded comments * edit serve command * restoring serve command * fixing serve conditional logic Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com> Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com> Co-authored-by: Daniel Choudhury <dannychoudhury@gmail.com> Co-authored-by: David Thyresson <dthyresson@gmail.com> * Fix paths so e2e tests link local framework correctly (#2345) * Adds process and require to eslint config web (#2354) * v0.31.0 * prepare branch for patch release v0.31.1 (#2420) * testing cli danny * updated * dp ftw * now it's dsp * Create github workflow to publish release candidate (#2399) Co-authored-by: David Price <thedavid@thedavidprice.com> * update github workflow files (#2419) Co-authored-by: Renan Andrade <renansoareess@gmail.com> * fix(generator): Page generators accept default options too (#2413) * fix(generator): Page generators accept default options too * Page generator also takes path positional * Fix command string with positionals * Remove DP testing branch deployment ;) * Empty commit to bump RC version * v0.31.1 * v0.31.2 * Add githead. Co-authored-by: David Thyresson <dthyresson@gmail.com> Co-authored-by: Sean Doughty <sean@render.com> Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com> Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com> Co-authored-by: David Price <thedavid@thedavidprice.com> Co-authored-by: Renan Andrade <renansoareess@gmail.com>

zakmandhro · 2021-05-10T05:20:20Z

Hi @dac09, @dthyresson: I just tried the main branch (0.31.2) and the page generator template source files are still .js instead of .tsx. Does this still need fixing? If so, I can do a PR.

Also, we now have generators returning inconsistent conventions.

Page generator uses plain functions (and uses any type for params!) and returns JSX using the return statement
Component and layout generator uses React.FunctionalCompoent (would be nicer if we just used :FC)
Cell generator returns JSX wrapped in parenthesis

Should we pick a convention and make it consistent?

dac09 · 2021-05-10T11:43:50Z

the page generator template source files are still .js instead of .tsx.

@zakmandhro yep happy for you to take a stab at this. While the template is JS, I don't think the TS version is any different (apart from file extension?)

I think I missed the case where you supply the param argument to the generator. What I would do probably... is to use the AvailableRoutes type thats generated on dev (in your project under .redwood/types.routes.d.ts) but can just be imported like this import {AvailableRoutes} from '@redwoodjs/router

Should we pick a convention and make it consistent?

This is a good point, open source is difficult :). Do you have a suggestion for which convention is best?

Thanks a bunch for offering to help!

Tobbe · 2021-05-10T12:02:09Z

Also, we now have generators returning inconsistent conventions.

Page generator uses plain functions (and uses any type for params!) and returns JSX using the return statement

Component and layout generator uses React.FunctionalCompoent (would be nicer if we just used :FC)

Cell generator returns JSX wrapped in parenthesis

Should we pick a convention and make it consistent?

Personally I do it like this whenever I can

export const MessagesOverview: React.FC = () => (
  <div>
    <div className="row">
      <div className="col-md-8 d-md-block">
        <MessageBox />
      </div>
      <div className="col-md-4 d-md-block">
        <MyMessagesList />
      </div>
    </div>
    <WaitingMessages />
    <LatestAnswersMessage />
  </div>
)

But I also understand why we'd generate components using this format

export const MessagesOverview: React.FC = () => {
  return (
    <div>
      <div className="row">
        <div className="col-md-8 d-md-block">
          <MessageBox />
        </div>
        <div className="col-md-4 d-md-block">
          <MyMessagesList />
        </div>
      </div>
      <WaitingMessages />
      <LatestAnswersMessage />
    </div>
  )
}

Because more often than not you'll end up adding a const [foo, setFoo] = useState() or something similar, and that's much easier/faster if your component is already generated using the latter format.

EDIT:

Ohh, and RW of course favors export default MessageOverview for some reason, instead of directly exporting the function.

…#2413) * fix(generator): Page generators accept default options too * Page generator also takes path positional * Fix command string with positionals

* feat(webhooks): Handle base64 encoded webhook payloads (redwoodjs#2340) * Handle base64 encoded payloads * Removes console logging. * Updates to Redwood-Render integration (redwoodjs#2336) * updating build and start abstractions * add boxen and improve setup deploy message * improve render.yaml format * removing region from static site * Handle focus on route change (redwoodjs#2321) * feat: focus RouteFocus on route change * feat: handle elements that aren't focusable * feat: add some tests for getFocus * style: split up tests * Fix "yarn rw test web" in e2e tests (redwoodjs#2329) * Copy rwjs packages to different location. * Run web tests. * Serve web/dist with `yarn rw serve web` (redwoodjs#2234) * Fix race condition in rwt link * Add new run command for api server * Update description for command * Rename run -> serve | Actually proxy the handler * Update packages/cli/src/commands/serve.js Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com> * PR Suggestions * Fix link * feat(rw-serv): Now supports serving web and both web+api * Regenerate yarn.lock from root * Refactor api-server file structure | Tests WIP * Update tests * Update packages/api-server/src/handler.ts Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com> * Read index content once, to prevent accessing FS every time Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com> * Verify signatures for secure webhooks and functions (redwoodjs#1843) * secureHandler verifies webhook signatures * Do not run verify as cannot acces env * Tests verify event. PR review updates. * WebhookVerificationError message is a string * Fixes Expected { after 'if' condition curly * Refactor secureHandler for a verifier * Get to green on ecureHandler.test * WIP verifier refactoring * Refactor into verifiers with types * Tests and implement verifyWebhook * Refactors signature header into options * Adds tests for none, secretKey, sha256 * Correct jsdocs * Adds ability to override event body payload * Refactor to use receiveAndVerify. Add tests. Support object payload for sha. * Add JWT verifier/Renames *Verifier & None to Skip * jwtVerifier wip * WIP * Add webhooks to api * Reorganize and add sha1 verifier * Adds more webhooks tests per verifier * Refactor createVerifier * Uses jwt.verify to check the issuer * Refactor circular dependency w separate types file * redwoodjs/api needs webhooks * Made VerifyOptions optional * Refactor WebhookVerifier interface w/ Danny * Cleanup jsdocs * Explains WEBHOOK_SECRET * Fix page generator taking in ts | Fix rw serve crashing out cli (redwoodjs#2335) * Fix page generator taking in ts | Fix rw serve crashing out cli * Change e2e to pass --typescript param * Remove unneeded comments * edit serve command * restoring serve command * fixing serve conditional logic Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com> Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com> Co-authored-by: Daniel Choudhury <dannychoudhury@gmail.com> Co-authored-by: David Thyresson <dthyresson@gmail.com> * Fix paths so e2e tests link local framework correctly (redwoodjs#2345) * Adds process and require to eslint config web (redwoodjs#2354) * v0.31.0 * prepare branch for patch release v0.31.1 (redwoodjs#2420) * testing cli danny * updated * dp ftw * now it's dsp * Create github workflow to publish release candidate (redwoodjs#2399) Co-authored-by: David Price <thedavid@thedavidprice.com> * update github workflow files (redwoodjs#2419) Co-authored-by: Renan Andrade <renansoareess@gmail.com> * fix(generator): Page generators accept default options too (redwoodjs#2413) * fix(generator): Page generators accept default options too * Page generator also takes path positional * Fix command string with positionals * Remove DP testing branch deployment ;) * Empty commit to bump RC version * v0.31.1 * v0.31.2 * Add githead. Co-authored-by: David Thyresson <dthyresson@gmail.com> Co-authored-by: Sean Doughty <sean@render.com> Co-authored-by: Dominic Saadi <32992335+jtoar@users.noreply.github.com> Co-authored-by: Peter Pistorius <peter.pistorius@gmail.com> Co-authored-by: David Price <thedavid@thedavidprice.com> Co-authored-by: Renan Andrade <renansoareess@gmail.com>

fix(generator): Page generators accept default options too

7891861

dac09 added the topic/generators-&-scaffolds label Apr 30, 2021

dac09 added this to the patch milestone Apr 30, 2021

dac09 requested a review from dthyresson April 30, 2021 18:17

dthyresson reviewed Apr 30, 2021

View reviewed changes

packages/cli/src/commands/generate/page/page.js Show resolved Hide resolved

Page generator also takes path positional

5cfa55c

dac09 added 2 commits May 3, 2021 17:11

Fix command string with positionals

fdf9477

Merge branch 'main' into fix/ts-page-generator

8172fed

dac09 merged commit d9b2cb9 into redwoodjs:main May 3, 2021

dac09 deleted the fix/ts-page-generator branch May 3, 2021 18:12

dac09 added a commit that referenced this pull request May 3, 2021

fix(generator): Page generators accept default options too (#2413)

1d8618b

* fix(generator): Page generators accept default options too * Page generator also takes path positional * Fix command string with positionals

dac09 added a commit that referenced this pull request May 3, 2021

fix(generator): Page generators accept default options too (#2413)

35c9e04

* fix(generator): Page generators accept default options too * Page generator also takes path positional * Fix command string with positionals

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(generator): Page generators accept default options too #2413

fix(generator): Page generators accept default options too #2413

dac09 commented Apr 30, 2021

github-actions bot commented Apr 30, 2021 •

edited

Loading

dthyresson left a comment

dthyresson commented Apr 30, 2021

dac09 commented May 3, 2021

dac09 commented May 3, 2021

dthyresson commented May 3, 2021

zakmandhro commented May 10, 2021 •

edited

Loading

dac09 commented May 10, 2021 •

edited

Loading

Tobbe commented May 10, 2021 •

edited

Loading

fix(generator): Page generators accept default options too #2413

fix(generator): Page generators accept default options too #2413

Conversation

dac09 commented Apr 30, 2021

What is it?

github-actions bot commented Apr 30, 2021 • edited Loading

📦 PR Packages

dthyresson left a comment

Choose a reason for hiding this comment

dthyresson commented Apr 30, 2021

dac09 commented May 3, 2021

dac09 commented May 3, 2021

dthyresson commented May 3, 2021

zakmandhro commented May 10, 2021 • edited Loading

dac09 commented May 10, 2021 • edited Loading

Tobbe commented May 10, 2021 • edited Loading

github-actions bot commented Apr 30, 2021 •

edited

Loading

zakmandhro commented May 10, 2021 •

edited

Loading

dac09 commented May 10, 2021 •

edited

Loading

Tobbe commented May 10, 2021 •

edited

Loading