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

Improved yarn rw console by adding persistent history #3221

Merged
merged 7 commits into from
Oct 15, 2021

Conversation

corbt
Copy link
Contributor

@corbt corbt commented Aug 9, 2021

A functional REPL really boosts developer productivity. Redwood already has a REPL (yarn rw console) but it's missing some functionality that could make it much more useful. This PR adds two features to the Redwood console:

  1. Persistent history. The Redwood REPL now stores history between sessions in a .redwood/console_history file. Old history can be navigated with the up/down arrows or searched with ctrl+r like normal.
  2. User-defined config. This PR adds a new api/src/config/console.[ts|js] file. Any code written in this file gets run when the REPL starts, and any exported constants are made available at the REPL top-level.

There are a couple other improvements I think would be useful that I may add to this PR if I have time, or add at some point in the future:

  1. Right now, project-relative imports (eg. import foo from 'src/lib/foo') don't work from the console.ts file. Probably something about the Babel config needs to change to support this?
  2. A top-level reload() function that reloads everything the user has imported to pick up new changes would facilitate a tighter REPL loop.

A functional REPL can really help DX and developer productivity. Redwood already has a REPL (`yarn rw console`) but it's missing some functionality that could make it much more useful. This PR adds two features to the Redwood REPL:

1. Persistent history. The Redwood REPL now stores history between sessions in a `.redwood/console_history` file.
2. User-defined config. This PR adds a new `api/src/config/console.[ts|js]` file. Any code written in this file gets run when the REPL starts, and any exported constants are made available at the REPL top-level.

There are a couple other improvements I think would be useful that I may add to this PR if I have time, or add at some point in the future:

1. Right now, project-relative imports (eg. `import foo from 'src/lib/foo'`) don't work from the REPL. Probably something about the Babel config needs to change to support this?
2. A top-level `reload()` function that reloads everything the user has imported to pick up new changes will facilitate a tighter REPL loop.
replContext.db = db
}

const loadUserConfig = (replContext) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

In the PR description, you mention api/src/config/console.[ts|js]. I was going to ask about using lib/ instead of config/, which I see you are using here. All good if so! Just reconciling the two things.

Instead of adding the file to the Create RW App template (not sure whether or not that what you are planning), what if this try/catch created the file if it doesn't exist?

  • console message to the user the file is created; link to doc how to use
  • check for language target using getProject().isTypeScriptProject from Structure package
  • and/or could create a new CLI command to create this file from a template (which we do for tsconfig.json as a rw setup ... command)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there might be some confusion, but the api/src/config/console path is the one the command looks for. The lib/ path I mentioned was just an example of the sort of project-relative import that is currently broken.

I didn't have concrete plans to create this file in the generator, but adding a creation step in the catch statement seems reasonable. I guess it comes down to whether you think most people who use the REPL will want to customize its behavior/set custom variables.

If we do end up autogenerating the user config file, then I'd probably move the binding of db into that file as well to show the user how that works and avoid the console using more magic than necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gah, you're correct. I confused the paths between db and console when I was (quickly) reviewing. config would be the "right" directory. Just thinking off the top of my head if there was a simplicity factor to consider... Regardless, ignore and move ahead!

I guess it comes down to whether you think most people who use the REPL will want to customize its behavior/set custom variables.

^^ I'd need to think on use-cases here. Dom might have more informed opinions.

If we do end up autogenerating the user config file, then I'd probably move the binding of db into that file as well to show the user how that works and avoid the console using more magic than necessary.

^^ +1 to this for sure. And feels like a great reason to create the file.

}

export const handler = () => {
// Transpile on the fly
babelRequireHook({
extends: path.join(paths.base, '.babelrc.js'),
extends: path.join(paths.api.base, '.babelrc.js'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this reminds me that @peterp is planning some re-working of Babel, including removing the default babelrc.js in the Redwood project. It will happen within a release or two.

I was going to suggest just removing this line now. However, projects might still have a manual implementation even if we suggest it's not best-practice. In that case, could they manage themselves via the api/src/lib/console.[ts|js]?

TBD

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this reminds me that @peterp is planning some re-working of Babel, including removing the default babelrc.js in the Redwood project. It will happen within a release or two.

FYI this rework can be found in #3231

@thedavidprice
Copy link
Contributor

Thanks for opening this PR @corbt! @jtoar started the console project and always intended it to be something for others to improve in time — he'll be excited to see this. Do know he's on vacation all week. Ping him next week if you don't hear from him first. I'll attempt to remind him as well.

I think your overall approach is 💯 Left a few comments that are for discussion.

Right now, project-relative imports (eg. import foo from 'src/lib/foo') don't work from the console.ts file. Probably something about the Babel config needs to change to support this?

^^ this definitely feels important to address. Dom might be able to help figure this out when he takes a look. Peter for sure, but he's a bit bandwidth-constrained at the moment.

A top-level reload() function that reloads everything the user has imported to pick up new changes would facilitate a tighter REPL loop.

^^ Dom just helped Peter with the (very) improved yarn rw dev command, including overhauling the watch-reload. He'll likely have some suggestions about approaching this and/or the possibility to use existing if applicable.

@jtoar
Copy link
Contributor

jtoar commented Aug 16, 2021

@corbt This is awesome! 🚀 Love the console file—writing code in a file that you can try out in the REPL is a wonderful workflow. ipython had a workflow like that that I've always missed. This’ll be great for learning.

project-relative imports

I think we can address project-relative imports by building the user’s project when the REPL starts up (we could use buildApi to do this programmatically) and then importing the built console file from dist instead of from src.

To get buildApi, you’d have to rebase, but since console’s currently broken in main, you should probably wait for #3231 to be merged (which should be soon I think).

At the same time, we should also generate the prisma client, and think about what to do when it goes out of sync. That could be another responsibility of the reload function.

console.[js|ts]

Because I’m not super sure about where the console file should go, here’s a little history on the api/src/config directory. The prisma db instance used to live there, instead of lib, but it was moved to lib in redwoodjs/create-redwood-app#47. Before that, it used to live in api/config redwoodjs/create-redwood-app#40. I don't think we've used api/src/config since, so this could be a great chance to do so. But what I'm also wondering is, because console is also kind of a playground/sandbox file, maybe that's not the right place for it? Two things that I should probably ask some core team members as well:

  • What do you think about moving the config directory to api, instead of api/src? To maintain symmetry with web, which has its config in web not web/src?
  • Should we have two files, one for configuring the REPL, and the other just for code that you're iterating on? Or would it be better to keep everything together? How do you normally work with a REPL?

I’m also a fan of moving the loading of the prisma client into the new console file as an example of how to use it / less magic. If we do that though, one thing that might be a bit burdensome is having to import your services. You could use the babel macro we've got:

import services from 'src/services/**/*.{js,ts}'

But if you just export that services object so that it's available in the REPL, it's not super straightforward:

> services
{
  users_users: {
    beforeResolver: [Function: beforeResolver],
    users: [Function: users],
    user: [Function: user],
    createUser: [Function: createUser],
    updateUser: [Function: updateUser],
    deleteUser: [Function: deleteUser]
  }
}

Here my model is User, and the return from the babel macro is a little too low-level. Maybe we could make a function, makeServicesForRepl, that handles this? I'll keep thinking!

Also, I'm happy to take on the work from here if you're pressed for time, just let me know!

@corbt
Copy link
Contributor Author

corbt commented Aug 17, 2021

project-relative imports

I think we can address project-relative imports by building the user’s project when the REPL starts up (we could use buildApi to do this programmatically) and then importing the built console file from dist instead of from src.

That makes sense. How much would it affect console latency? There's already a ~5 second lag to interactivity on my machine when I run yarn rw console; which seems surprisingly high. But if the reload() function is solid and relatively fast that's probably fine, since I can just keep a persistent console open indefinitely.

To get buildApi, you’d have to rebase, but since console’s currently broken in main, you should probably wait for #3231 to be merged (which should be soon I think).

Cool, looks like that's merged now. I was working off the v0.35.2 tag since I've been running my projects off the tagged releases, not head, but I assume it wouldn't be too hard to start working off head instead.

At the same time, we should also generate the prisma client, and think about what to do when it goes out of sync. That could be another responsibility of the reload function.

Agreed, reload() should regenerate the prisma client if the schema has changed.

  • What do you think about moving the config directory to api, instead of api/src? To maintain symmetry with web, which has its config in web not web/src?

I know this question isn't directed at me, but I agree, that seems like a more natural place to keep this. I just went with api/src/config since that was the path already in paths.ts; I didn't realize we weren't using it for anything!

  • Should we have two files, one for configuring the REPL, and the other just for code that you're iterating on? Or would it be better to keep everything together? How do you normally work with a REPL?

Personally I'd rather have a single blessed console.ts file with all the REPL configuration. Generally the code I'm iterating in already lives somewhere else in my api/src/ directory, so I just import it from wherever it lives and then re-export it within the console.ts file.

If a user really wants to separate out "user code" from "console config", they can always create another file api/config/userConsoleCode.ts file and import it from api/config/console.ts.

I’m also a fan of moving the loading of the prisma client into the new console file as an example of how to use it / less magic. If we do that though, one thing that might be a bit burdensome is having to import your services. You could use the babel macro we've got:

I don't have enough context to know the right way to import these services, but agree that making them available from the console by default would be awesome. That could be extended to be even more magical if we just import all user code for you. It would be fantastic if from the console I could just call services.users.users.createUser(...) or lib.util.myUtilFunction(...) without having to explicitly import those modules. Of course, reload() would have to re-import those modules every time to pick up changes.

Also, I'm happy to take on the work from here if you're pressed for time, just let me know!

Sure, I'd be thrilled to pass this off to someone with more context if you're interested in working on it. :)

@jtoar
Copy link
Contributor

jtoar commented Oct 15, 2021

Hey @corbt, sorry that it’s been so long since I’ve updated you on this one. I brought this PR up recently at a core team meeting and we had a great conversation about it; we actually want to talk about it more! But the bad news is that we decided to table those conversations for now because we really need to ship a v1 rc.

The one thing we all agreed on was the history functionality, so I'll merge that part of this PR in today. We do envision the rest of these features coming to the console, just at a time when we have less mission-critical features to work on / bugs to squash. And it'd be easier if our babel config settled down a bit!

I think the reason we had to table the discussion was that it became too large in scope; we started asking questions like "what is the console for? Is it just a database shell? If we make an app's Service's available, isn't it way more than that?" and we knew nailing down answers to those kinds of questions would take a bit of time.

Adding another file to the redwood app structure also made us question some of the existing file structure; e.g., if the console file is in config, why aren't the other config files (jest, jsconfig/tsconfig, etc) there too? We didn't have an immediate answer to that one either.

One thing that would be super valuable right now is your answer to the what-is the-console-for question. Do you use it to iterate only on database-related things?

@jtoar jtoar merged commit 6b59122 into redwoodjs:main Oct 15, 2021
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Oct 15, 2021
@thedavidprice
Copy link
Contributor

@jtoar is there an open Issue collecting these learnings + ideas for next steps here? If not, would be great to have.

@thedavidprice thedavidprice changed the title Improved yarn rw console Improved yarn rw console by adding persistent history Oct 16, 2021
@corbt
Copy link
Contributor Author

corbt commented Oct 18, 2021

the bad news is that we decided to table those conversations for now because we really need to ship a v1 rc.

Understandable. There's a lot of other great stuff going on in Redwood!

One thing that would be super valuable right now is your answer to the what-is the-console-for question. Do you use it to iterate only on database-related things?

I use the console a lot. One common pattern when I'm working in Rails, which has a very full-featured dev console, is to change a method on a service or model, run reload! in my console, and try running the method again. I do that iteratively until the method does what I want. I also do that with smaller chunks of code -- I'll keep a console open and copy in pieces of a method I'm trying out to make sure each step works as expected (or iterate if it doesn't) until the full method is working correctly.

@thedavidprice thedavidprice modified the milestones: next-release, v0.38.0 Oct 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants