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

feat(dart_frog_cli): add dart_frog_new_brick for routes #623

Merged
merged 1 commit into from
May 12, 2023

Conversation

renancaraujo
Copy link
Contributor

@renancaraujo renancaraujo commented Apr 27, 2023

Status

READY

Description

Extracted from #616, adds the "new" brick with the implementation for route creation

This is the bulk of #260

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

Copy link
Contributor

@alestiago alestiago left a comment

Choose a reason for hiding this comment

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

Excited to get this merged, I left some comments. Let me know what you think! Besides, I also tested the tool this are some interesting findings:


Given the file structure:

hello
-- index.dart
-- world.py # Not a dart file!

Then doing mason make dart_frog_new world.

I get as expected:

hello/
-- index.dart
-- world.py
-- world.dart

Do we have (or eventually within e2e) a test to verify this behaviour?


Given the file structure:

hello[id].dart

Then doing mason make dart_frog_new hello[id]/ee[id].

Gives:

hello[id]/
-- index.dart
-- ee[id].dart

The content of ee[id].dart is not valid Dart code, since it has two declarations with the same name.

import 'package:dart_frog/dart_frog.dart';
Response onRequest(
  RequestContext context,
  String id,
  String id,
) {
  return Response(body: 'Welcome to Dart Frog!');
}

Given:

hello/
-- index.dart

Doing mason make dart_frog_new hello.

Fails as expected but immediately after reporting:

Failed to create type: /hello already exists.
✗ Making dart_frog_new (15ms)
type 'Null' is not a subtype of type 'Object' # This line is not a nice error message

Is the last error line intended?

bricks/dart_frog_new/README.md Outdated Show resolved Hide resolved
bricks/dart_frog_new/hooks/lib/pre_gen.dart Outdated Show resolved Hide resolved
bricks/dart_frog_new/hooks/lib/pre_gen.dart Outdated Show resolved Hide resolved
bricks/dart_frog_new/brick.yaml Show resolved Hide resolved
bricks/dart_frog_new/__brick__/{{filename}} Outdated Show resolved Hide resolved
bricks/dart_frog_new/hooks/lib/pre_gen.dart Show resolved Hide resolved
@renancaraujo renancaraujo force-pushed the renan/dart_frog_new_brick branch 5 times, most recently from 012507b to 2bdcba8 Compare May 10, 2023 16:18
@renancaraujo
Copy link
Contributor Author

Do we have (or eventually within e2e) a test to verify this behaviour?

Goo idea, will add to the doc

@renancaraujo
Copy link
Contributor Author

The content of ee[id].dart is not valid Dart code, since it has two declarations with the same name.

Good catch, added a ceghck for that, will add to the doc as well

@renancaraujo
Copy link
Contributor Author

Closing and reopening to kick actions

@renancaraujo
Copy link
Contributor Author

Is the last error line intended?

It is not, but it happens with you use Mason directly since it always runs the brick and postgen even if pregen fails. That doesn't happen when integrated with our cli.

@renancaraujo renancaraujo merged commit 8543fdb into main May 12, 2023
4 checks passed
@renancaraujo renancaraujo deleted the renan/dart_frog_new_brick branch May 12, 2023 15:52
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.

None yet

2 participants