-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Incremental Builds #130
Comments
For live editing using the server fsnotify tells you which files have changed so you could filter down to just the file itself and avoid removing/recreating files without changes. That can break where one file's changes influences another. |
This branch is a first attempt at doing the local dev all in memory using a Saving is not bad vs full initial build.
No doubt a multitude of bugs and there is excess code but a starting point at least. |
@padraicbc I've been playing around a bit with this today and it's insane! Especially when you run it with The regular
I know you were showing me some gopack optimizations, maybe it's related? I've also updated gopack recently to make it work with Svelte I tested your mem branch on a bunch of different repos with the When running on https://github.com/jimafisk/plenti_multi-pager_example I got:
And on https://github.com/jimafisk/plenti_theme_test_bigspring it threw:
There are a few other things I was planning on working on this weekend, but I don't want master to diverge too much from all the work you're doing here and make merging a pain. Do you think we should pull your updates in and work together to squash these bugs? |
That was me stupidly doing too much storing without checks. There is still some logic to be ironed out in relation to which node module files shpuld be stored in mem(if any) to avoid repeated work in gopack but it should all be good now.
I need to add a check to verify that we had hashed/seen the layout before. checking hash was > 0 fixes it
I still have to add the logic for using theme folders. Having the example to use will make it trivial to add. All bar themes should be good now. They worked locally for me anyway.... I will add the theme logic and tidy up a few pieces but happy enough to pull in whenever you want. The more I thought about it, the more it seemed that all we care about is storing and serving bytes, we don't care about directories or stating anything. Nothing bar maybe walk would be useful in terms of a filesystem but that can be replicated more efficiently so a map and a type that implements ServeHTTP that serves our specific needs is all we actually need. |
For the themes I am getting |
So |
Where should the node_modules live? |
It could get messy how this is currently set up, but potentially Is that making sense? Basically instead of building from the project root like we normally do, we combine all the files ( Edit: It's possible that I've messed something up somewhere along the line, but that's ^ how it works in theory. |
I think the issue is |
That definitely makes sense, it's possible that was never updated to be theme aware by passing the |
Scratch that, I get it now. |
That should pull in all the project files to overwrite the theme files that have already been copied over. We're purposely excluding the theme itself and other files that aren't needed from the build. This video at around the 37 minute mark helps explain that inheritance process: https://youtu.be/dT69Ph2XkjQ?t=2223. |
If we are using the theme logic do we still need to copy assets over as we walk over the assets in merge? |
The way I envisioned this is you can inherit full sites (layout, content, assets), but you can also excluded any of those aspects as demonstrated here: https://youtu.be/dT69Ph2XkjQ?t=2161. I imagine lots of folks will just want to inherit svelte templates and leave behind all the json files and any assets like .png, .jpg, etc. You can do that by adding an |
I get the general logic. I think the temp_build is f**king with my brain tonight! Try out the latest commit to see how it works when you get a chance. I will watch the vid during the week and use the theme to get a better understanding of how it all fits together. Once the rest is working well it should be easy enough to add the theme logic. Especially when i understand it! |
Awesome I'll check it out! The |
@jimafisk if we are using a theme, can we just iterate over the theme dir and add any assets/layout/content |
Also should we not be specifically be copying over just the content, layout, ejected, node_modules and assets folders from any theme? |
Hey @padraicbc, you're right that
|
I get the advantages of keeping it separate but it does make building a lot more complex, especially doing it in memory. The more themes you add the worse it gets. Also If you have conflicting filenames like the video example then it would probably be better to stop the build and ask a user to confirm or you end up with potentially very strange bugs. |
Another option would be to put them under layout/theme/somename/.. content/theme/ssomename/... which still keeps them separate but is the whole point of copying over from themes into a build folder just to allow overwriting or why not just compile from where it is? |
In this case the preference would go to the project root. It would be the user's responsibility to name things accordingly if they do not want to override theme assets.
This could be a harder challenge. I wish it were as simple as preference goes to the project root, but of course different versions of node_modules don't always have the same files that line up one-to-one. So you may end up with a
If it's not tenable, for now do you think it would make sense to just running theme builds on the filesystem? Basically if you're working with themes, don't pass the |
I had it working 95% in memory but it was really ugly. I think tidying up the logic as is and creating the "Builder" type we chatted about the other day to allow easier access to config plus removing node would make implementing a lot easier, so maybe wait till then? |
Yeah that makes a lot of sense. We're carrying around a lot of cruft with the node builds and some other old stuff that's just lingering. It will be nice to clean house a bit. When you're ready let's get this in and we can work off that for the next release. Thanks for plowing ahead with this! |
No worries. Probably better get feedback with the default setup and squash any obvious bugs before adding it to themes anyway. I will add a PR later in the week. The last commit fixed the other issues? |
I just ran through all the testing sites again with your latest commits and everything seems to be working now, even the themes example - is that expected? |
The themes don't actually work properly though no? |
It seems to work ok using https://github.com/jimafisk/plenti_theme_test_bigspring. I didn't dig too deep but the build isn't throwing errors and the site navigation is functional. |
It wasn't working for me. I thought I had to still manipulate the path prefix for some parts but maybe it is the version of svelte I am using as I installed with npm after cloning. It can/will work fine once we keep the temp_build folder. What I was working on was trying to also keep that in memory which wasn't as straightforward. Are all the routes working for you including when visitied directly? |
The routes that exist, yes. That might be part of the problem, that theme isn't complete so the pricing, contact, and faq pages won't work because they don't exist. |
Client side routing is broken for me but it is definitley a svelte issue . There a couple more issues and things I am not sure on.
The fact we don't watch the themes folder, if a user wants to edit a lot of the files it's best just to clone the theme and use that as the root so rebuilding etc.. works on file changes? |
Yeah good call. I just changed some of this code to remove
This is a great point. I'm thinking for deps we should probably just manage this at the root level like you've been saying. If you're using a theme that relies on custom npm configuration, it should be your responsibility to copy that config to your project root and do any necessary installing there. Would you agree?
Yup that's exactly right. Theme files should not be edited directly so you can continue to pull upstream changes without overriding your work.
Yeah that's definitely an option. If a user plans to override every file in a theme, it might make sense to not use it as a "theme" at all - just clone it and work off it like a normal project. |
That works fine for me anyway. You could automate it somewhat but conflicting packages adds to the complexity so I think it is fair to ask a user to update the deps themselves. So essentially one All good on the rest. |
Yeah that all sounds good! If the plenti/cmd/build/npm_defaults.go Line 22 in e666beb
Of course if you're using a theme that requires something custom, it's on you to make sure you have it in your |
Sorry, yes it was the modified build path for the theme that caused the error not at root. |
@jimafisk A few more hours will have this ready but I will wait until v0.4 is out to comlete it. Should have first go at incremental disk builds added also by then. |
That's awesome, it's going to make a huge difference when we pull that in. I should have the v0.4 stuff done tonight. |
@padraicbc amazing work on this! Your PR is in v0.4.13 and the in-memory stuff is crazy fast on my computer. Are you using this issue for incremental disk build stuff you're working on, or should I close this out? |
No prob, you can close it out. The disk inc should be a whole a lot simpler as it's just paths/hashes stored with a little more info in a JSON object. I should have that added in next few days. |
That sounds amazing, I'm very interested in the ANTLR stuff you're working on. I'm still trying to wrap my head around how the golang target for ANTLR even works. My understanding is antlr4 is written in java, does this come with some of the same challenges as cgo or is it fundamentally different? Just let me know how I can support your efforts! |
The ANTLR Java binary takes in the lexer/grammar files and spits out java visitors/listeners depedning on what you pass. It also supports a few other languages. To get visitors for go you woukd Java is only used to parse the grammar/lexer files and output the Go files. After that it is all Go so no calling into Java. It would completely remove the dependency on CGo/v8 so while there is room for improvement with the ANTLR Go runtime, it would still be light years faster, and greatly simplify the release builds. For the import parsing.I am not 100% on the logic from here down. The ANTLR parser will pull any imports/exports which can easily be replaced/manipulated/removed as you can get access to the start/end offsets for each statement. The Gopack import search could also be done in a similar fashion which would be more robust than the regex. |
Don't let me distract you too much, I'm mainly just thinking out loud. I'll spend a little time learning the basics of ANTLR and try to get up to speed on the principals of compiler design.
I assumed we'd use an existing
So it sounds like we would run this on the current ecmascript spec, and potentially have to run in it the future if the spec changed, but this would not be run often (i.e. every time we build plenti).
Do you plan on contributing back to https://github.com/antlr/antlr4/tree/master/runtime/Go/antlr or keeping https://github.com/padraicbc/antlr4 as a more permanent fork? If it's helpful for me look at anything in particular, or even try to find people who are better versed in this than myself to contribute just let me know. I know that sometimes too many cooks can slow things down though.
🤤 You have my buy-in. Cgo is proving challenging esp as new architectures like arm64 for m1 are added, windows support would be nice too.
The "removing" of static import/export statements is really just wrapping these lines in comments because v8 would blow up when it tried to read them. So instead of reading across files by following their imports, we dump all components into the same ctx so they can reference each other. To avoid naming conflicts, we just make sure that each has a unique component signature (e.g.
I think a lot of this nonsense could go away if we start parsing components properly vs massaging them to work in v8. If we compiled the components with Go directly, I'm not sure if we'd need Gopack in the same capacity we have it currently. Navaid doesn't change all that often so we could potentially build that into our ejectable core and if folks need to include node_modules for something specific to their project, we could leverage the esbuild api for a more robust experience.
We'd need a way to comprehend htmlx control structures, or we could even reinvent these a bit. Also scoping css to the markup provided in the component + shaking out unused styles from the bundle. I'm honestly not sure I get the full picture of how things like svelte's reactive declarations would work, but maybe I'm getting ahead of ourselves a bit. |
Yes, and for now both .g4 files are identical to the ANTLR repo but there are some errors that need fixing so they will be different at some stage!
Yep, a few fixes to be added like those above and some manual work of actually using the new Visitor/Listener . A 10000ft view of the genral flow when using a visitor: A few types of interest:
TLDR; Just embed It is a slightly complicated world of satisfying interfaces that can likely be improved upon but once setup then re-running to generate for any new ecma changes is just a matter of implementing the new methods for the new types/changes. All the rest should generally remain unchanged bar maybe a new select case or two .
Famous last words ... but I think the htmlx will be the easiest part It has a very llimited grammar outside of standard html. We could likely adapt the builtin html parser or just use ANTLR again. One of the svelte devs has a short video that gives an overview of how things work. This is also a really nice lecture series on ANTL4. The implementaion varies vs how it works in Go but it gives you a really solid undestanding of how to work with all parts of ANTLR. I am going to walk through the svelte compiler code and implement the ts types in Go. I will add some pseudocode to how the types get used and try to buld up the flow of how it all works. Then it is a matter of moulding the parser/compiler to output what is needed. I am happy for anyone to help... |
@padraicbc you're next level 😳💨
I'm digging into the materials you sent so I can get myself up to speed at a basic level. I wonder if my time would be best spent trying to find someone with the skill set to hit the ground running with you. Let me know if it's helpful to connect about specific tasks you need done or strategy for getting eyes on the project. |
I started porting this over to Go to try out integrating the Parser. You can see an import ImportDeclaration tyope is used, the definition is here. Taking
It is actually a lot easier than it seems once you have the maze of interface satisfied.The grammar shows you what is available on each node/context. A large part of what is needed is creating equivalent objects in Go that you see in the typescript code like |
Ok nice, that makes sense to me at a high level. Let's hop on a quick call next week, I'll send you a calendar invite. Just let me know if another day/time works better and we can coordinate in email. I'll close this issue for now, but feel free to brainstorm or share ideas in this queue! |
From @padraicbc
The text was updated successfully, but these errors were encountered: