amberframework / amber

A Crystal web framework that makes building applications fast, simple, and enjoyable. Get started with quick prototyping, less bugs, and blazing fast performance.
https://amberframework.org
MIT License
2.58k stars 206 forks source link

feat!: remove webpack from dependencies #1304

Closed drujensen closed 1 year ago

drujensen commented 1 year ago

Description of the Change

This change removes webpack from the dependencies. It also upgrades bootstrap to 5.0 using cdn links in the main template.

Alternate Designs

Keep it as is

Benefits

Removing webpack, nodejs, and npm dependencies simplifies amber templates and decouples from the developers choice on frontend solutions.

Possible Drawbacks

This is a BREAKING CHANGE and requires a MAJOR version upgrade.

drujensen commented 1 year ago

You might be right about the breaking change. It still a significant change that we should look through the documentation and see if we need any updates. I will work on fixing the tests and adding htmx for delete functionality next.

drujensen commented 1 year ago

@crimson-knight do you know if the tests passing in master? I'm getting all kinds of errors:

crystal spec spec/amber/pipes/powered_by_amber_spec.cr:19 # Amber::Pipe::PoweredByAmber Adds X-Powered-By: Amber to response should contain X-Powered-By in response
crystal spec spec/amber/pipes/pipeline_spec.cr:34 # Amber::Pipe::Pipeline with given server config routes
crystal spec spec/amber/pipes/pipeline_spec.cr:41 # Amber::Pipe::Pipeline with given server config responds to GET request
crystal spec spec/amber/pipes/pipeline_spec.cr:48 # Amber::Pipe::Pipeline with given server config responds to PUT request
crystal spec spec/amber/pipes/pipeline_spec.cr:55 # Amber::Pipe::Pipeline with given server config responds to PATCH request
crystal spec spec/amber/pipes/pipeline_spec.cr:62 # Amber::Pipe::Pipeline with given server config responds to POST request
crystal spec spec/amber/pipes/pipeline_spec.cr:69 # Amber::Pipe::Pipeline with given server config responds to DELETE request
crystal spec spec/amber/pipes/pipeline_spec.cr:76 # Amber::Pipe::Pipeline with given server config responds to HEAD request
crystal spec spec/amber/pipes/client_ip_spec.cr:11 # Amber::Pipe::ClientIp IP from headers gets first client IP from default header
crystal spec spec/amber/pipes/client_ip_spec.cr:28 # Amber::Pipe::ClientIp IP from headers gets client IP from first custom header found
crystal spec spec/amber/cli/commands/init_spec.cr:94 # Amber::CLI::MainCommand::New Database settings generates pg correctly
crystal spec spec/amber/cli/commands/exec_spec.cr:26 # amber exec within project executes multi-lines from the command-line argument
crystal spec spec/amber/cli/commands/exec_spec.cr:39 # amber exec within project executes a .cr file from the first command-line argument
crystal spec spec/amber/cli/commands/exec_spec.cr:47 # amber exec within project opens editor and executes .cr file on close
crystal spec spec/amber/cli/commands/exec_spec.cr:53 # amber exec within project copies previous run into new file for editing and runs it returning results
crystal spec spec/amber/cli/commands/exec_spec.cr:62 # amber exec outside of project executes outside of project but without including project
crystal spec spec/amber/cli/commands/database_spec.cr:57 # database postgres test has test  connection settings

For example the PoweredByAmber Pipe is not passing and it looks like its not configured. Let me know if I need to fix these first.

crimson-knight commented 1 year ago

@drujensen yes I have 100% passing tests on master on my local when I run bin/amber_spec. The GithubActions fail for the redis related tests because I couldn't get the redis configuration to work.

drujensen commented 1 year ago

which version of crystal are you running?

crimson-knight commented 1 year ago

The latest 1.6.2. All tests passed originally when I updated everything on 1.5 though

drujensen commented 1 year ago

ok, maybe its crystal 1.6.2? I can't get them to pass locally. I will track them down. Thanks.

crimson-knight commented 1 year ago

It's passing for me with 1.6.2. I think a large part of it is around the DB being changed from PG to sqlite. There are a lot of tests around PG being the default DB and relying on that being present.

drujensen commented 1 year ago

not related. I'm trying to get master to pass locally. no worries. I will figure it out. Thanks for checking.

crimson-knight commented 1 year ago

No worries. I'll be putting around tomorrow working on my bulldozer if you need anything, just message me in Discord

drujensen commented 1 year ago

Looks like the redis tests are failing in github actions. I've open an issue #1305 to address that.

marccremer commented 1 year ago

You might want to move everything in assets into public at the moment we do not have the amber logo when we start a new project

drujensen commented 1 year ago

@marccremer Thanks. I will look into it. I moved the logo into the public/images/logo.png but I will confirm I didn't miss anything.

Or are you saying for the amberframework.org website?

marccremer commented 1 year ago

This is what you see when you start a new project 2022-11-14-162635_1055x482_scrot

drujensen commented 1 year ago

@marccremer nice catch! 💯 Looks like I missed adding these changes back to the templates. Fixing...

drujensen commented 1 year ago

@marccremer should be fixed: Screenshot 2022-11-14 at 7 47 43 AM Screenshot 2022-11-14 at 7 48 10 AM

marccremer commented 1 year ago

Did you push your changes? These commits do not touch the relevant files

drujensen commented 1 year ago

strange. checking .gitignore. maybe i need to explicitly add them.

drujensen commented 1 year ago

ok, found cli in the .gitignore which explains why my commits were not pushing. This should be fixed now.

marccremer commented 1 year ago

lgtm

crimson-knight commented 1 year ago

@drujensen FWIW I had added the CLI binary to the .gitignore because it's compiled as arch specific, and not everyone is on the same architecture. I've had a fair amount of people on linux/intel mac and myself on M1 mac trying to run the cli tool in dev, so my thought was to ignore the CLI binary and always recompile after pulling changes to ensure you have the correct binary.

May not matter since you'd get an error trying to run a binary for an arch you don't have and you'd know to re-compile anyway

drujensen commented 1 year ago

@crimson-knight makes sense. The cli entry also excluded the whole cli directory so everything that was being added under that directory was being ignored. probably just need to specify the binary file instead.

crimson-knight commented 1 year ago

Woops! :) I thought I had specified only the binary and ensure it's compiling into the bin directory