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

[Bug?]: Scaffold does not produce src/lib/formatters #6840

Open
1 task
AlvaroPata opened this issue Nov 9, 2022 · 10 comments
Open
1 task

[Bug?]: Scaffold does not produce src/lib/formatters #6840

AlvaroPata opened this issue Nov 9, 2022 · 10 comments
Labels
bug/needs-info More information is needed for reproduction

Comments

@AlvaroPata
Copy link

What's not working?

When generating a scaffold the resulting cells have an import from src/lib/formatters that does not exist.

Screenshot 2022-11-09 at 17 13 42

How do we reproduce the bug?

yarn rw g scaffold portfolio

What's your environment? (If it applies)

System:
    OS: macOS 12.6
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.15.1 - /private/var/folders/37/hrj2n25s0szb0w7_wg4v0yw80000gn/T/xfs-bed756c4/node
    Yarn: 3.2.3 - /private/var/folders/37/hrj2n25s0szb0w7_wg4v0yw80000gn/T/xfs-bed756c4/yarn
  Databases:
    SQLite: 3.37.0 - /usr/bin/sqlite3
  Browsers:
    Chrome: 107.0.5304.87
    Firefox: 106.0.5
    Safari: 16.0
  npmPackages:
    @redwoodjs/core: ^3.3.1 => 3.3.1

Are you interested in working on this?

  • I'm interested in working on this
@AlvaroPata AlvaroPata added the bug/needs-info More information is needed for reproduction label Nov 9, 2022
@Josh-Walker-GM
Copy link
Collaborator

Hi @AlvaroPata I'm trying to reproduce this. Could you share your schema for portfolio that is in api/db/schema.prisma?

I created a new project and ran yarn rw g scaffold userexample. Two files then had imports from src/lib/formatters and they were fine. I also note that all of those functions were present in my src/lib/formatters.

Do you have a src/lib/formatters file? If so does it export any functions?

@AlvaroPata
Copy link
Author

AlvaroPata commented Nov 10, 2022

Hi @Josh-Walker-GM ! thanks for checking it out

This is the schema for portfolio:

model Portfolio {
  id         String     @id @default(cuid())
  type       AssetType
  properties Property[]
  tenant     Tenant     @relation(fields: [tenantId], references: [id])
  tenantId   String
  assets     Asset[]

  @@unique([tenantId, type])
}

I first generated standalone SDLs with yarn rw g sdl portfolio and worked on the service logic and tests.

When I tried to scaffold the rest of the files with yarn rw g scaffold portfolio, as expected, the sdls and services didn't get overwritten, since they already existed, and the rest of the scaffold–on the web side–got generated, just not the src/lib/formatters; it doesn't exist.

This is the content of the web folder, as you can see there's no src/lib/formatters to be found.

web/
├── jest.config.js
├── package.json
├── public
├── src/
│   ├── App.tsx
│   ├── Routes.tsx
│   ├── components/
│   ├── index.css
│   ├── index.html
│   ├── layouts/
│   ├── pages/
│   └── scaffold.css
└── tsconfig.json

Did I do something wrong?

Thanks

@AlvaroPata
Copy link
Author

I just run yarn rw g scaffold with a new model that did not have an sdl yet and the lib with the formatters got created.
So I guess that was the problem.

I s that the intended behaviour?

@Josh-Walker-GM
Copy link
Collaborator

Thanks for the extra details! I'll take a look later today to see if I can spot where we go wrong.

My first impression is that we definitely shouldn't be generating files that contain missing imports. From a DX perspective we should either generate it all or error out and let you know we can't do what you want.

@Josh-Walker-GM
Copy link
Collaborator

Hi @AlvaroPata, so I've just began running through the steps you described. I'm using the existing userexample model since it already comes with the default project but otherwise I've followed your steps of first generating the sdl and then the scaffold.

When I run this the scaffold errors out when it encounters the pre-existing .sdl.ts file. I've included a screenshot of the console below. Did you also see this or did everything on the console appear to be good?

image

I do end up with import { } from 'src/lib/formatters' within my components/UserExample/UserExample/UserExample.tsx which is only a warning rather than an error (because I don't actually import any non-existing functions) but begins to validate your issue.

@AlvaroPata
Copy link
Author

Hi @Josh-Walker-GM, yeah that's exactly what I saw!

@Josh-Walker-GM
Copy link
Collaborator

Thanks for clarifying @AlvaroPata.

In an ideal world we would support reverting our generators actions when an error occurs - there are some open issues discussing this. If we did support this then you'd have seen this error and then not have had any half functioning code generated. There are also some open issues on linting generated files which would have detected my example generated code which had the empty import.

Was there any particular reason you chose to generate the sdl separately to start with before a full scaffold - just out of interest? If it's a common way to want to work then I can imagine we might want to support this. By for example asking if you want to skip certain steps if you've already implemented them. Your issue raises good thoughts for sure!

Am I right in saying this issue isn't blocking any progress for you on your projects? As you said you were able to generate the associated files by scaffolding out a different model.

@AlvaroPata
Copy link
Author

Hi @Josh-Walker-GM !

Was there any particular reason you chose to generate the sdl separately to start with before a full scaffold - just out of interest?

In this particular case, I wanted to focus on the business logic without creating all the related frontend files. Working on the service layer helps me further develop and refine the underlying schema. Since I wouldn't be using these files, it wouldn't make sense to have a lot of pages, cells, components, etc at this point: It would only add unnecessary complexity. I would like to work on the frontend side once the business logic has been developed and tested.

Is there a better workflow? I'd be happy to try it out!

Am I right in saying this issue isn't blocking any progress for you on your projects? As you said you were able to generate the associated files by scaffolding out a different model.

Yes, you're right. Thank you for your time!

@Josh-Walker-GM
Copy link
Collaborator

Is there a better workflow? I'd be happy to try it out!

Nope that workflow sounds sensible to me and if it's the one you're comfortable and productive with then stick with it I'd say.

I'll raise some discussion on how we can improve this sort of situation in the future. Some of the thoughts that come to mind about potential improvements:

  • Do our error messages need to be clearer?
  • Should we let users have options like --skip-sdl when using the scaffold generators?
  • Should we prompt a user when we detect they already have an SDL file and let them decide if it's okay to continue with the scaffold generator?
  • Should we prioritise our generators reverting on an error?
  • Should we priorities linting our generated code?

Thanks for getting involved and helping me understand what went wrong.

@AlvaroPata
Copy link
Author

AlvaroPata commented Nov 15, 2022

Having a way to integrate the scaffold generator with the standalone generators (e.g. SDLs, components, cells...) would be great. Generating these files separately–especially on the web side–leads to an inconsistent folder structure, which doesn't follow the logic of the scaffold, but that is a different subject.

All of your suggestions are really good! Prompt, flag, better error message and even the possibility to revert the generated files be really useful.

In case these files cannot be created unless the SDLs have not been created yet, it would be great to have a clearer error message.

Maybe a generator for the formatter library would also be helpful? I don't know, just an Idea...

In case these files cannot be created unless the SDLs have not been created yet, it would be great to have a clearer error message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/needs-info More information is needed for reproduction
Projects
Status: Triage
Development

No branches or pull requests

2 participants