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.

Sign up for GitHub

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

Jump to bottom

Feature/issue 1007 api routes #1017

Merged
merged 20 commits into from
Dec 17, 2022
Merged

Feature/issue 1007 api routes #1017

merged 20 commits into from
Dec 17, 2022

Conversation

thescientist13
Copy link
Member

@thescientist13 thescientist13 commented Dec 3, 2022 •
edited

Related Issue

resolves #1007

Summary of Changes

  1. Introduce support for src/api/* based routes that confirm to a Request / Response contract (like will also happen for Standardize on Web APIs (Request / Response / URL / etc) #948)
  2. Add test cases and created "API Routes" docs page

TODO

  1. Doesn't change on refresh - ESM caching issue I believe :/ - commented in Invalidate cache when using import nodejs/node#49442 to show our support for "native" ESM cache busting
  2. Validate build / serve commands
  3. Flesh out entire Request / Response objects
  4. Port is hardcoded
  5. response assumes .json()
  6. stop using path / url helpers!
  7. Testing
  8. Documentation
  9. new API serve specs are running, but not finishing :/
  10. Remove demo code

Note: more of the Request/ Response will get fleshed out as part of #948

Questions

  1. Should this be src/api/* or src/pages/api/* (like in Next) - will start with src/pages for now
  2. Should we add APIs to the graph and / our output some stats? That will definitely blow up our GraphQL schema as is, so might need to be its own task - tracked in bundle and optimize server and API routes and serve from output directory (decouple serve task from source code) #946
  3. What if there is just an /src/api/* directory and no /src/pages/*, should we not emit default index.html / 404.html? (e.g. for an entirely serverless stack) - tracked in bundle and optimize server and API routes and serve from output directory (decouple serve task from source code) #946

@thescientist13 thescientist13 added documentation Greenwood specific docs CLI feature New feature or request labels Dec 3, 2022
@thescientist13 thescientist13 self-assigned this Dec 3, 2022
thescientist13
thescientist13 commented Dec 6, 2022
View reviewed changes
packages/cli/src/plugins/resource/plugin-api-routes.js Outdated

// https://github.com/nodejs/modules/issues/307#issuecomment-1165387383
if (process.env.__GWD_COMMAND__) { // eslint-disable-line no-underscore-dangle
href = `${resolvedUrl}?t=${Date.now()}`;

This comment was marked as outdated.

Sign in to view

packages/cli/src/plugins/resource/plugin-api-routes.js Outdated
let href = new URL(`${this.getBareUrlPath(url).replace('/api/', '')}.js`, `file://${this.compilation.context.apisDir}`).href;

// https://github.com/nodejs/modules/issues/307#issuecomment-1165387383
if (process.env.__GWD_COMMAND__) { // eslint-disable-line no-underscore-dangle

This comment was marked as outdated.

Sign in to view

@thescientist13 thescientist13 changed the base branch from master to release/0.28.0 December 10, 2022 19:30
@thescientist13 thescientist13 changed the base branch from release/0.28.0 to master December 10, 2022 19:36
@thescientist13 thescientist13 changed the base branch from master to release/0.28.0 December 10, 2022 19:39
@thescientist13 thescientist13 marked this pull request as ready for review December 14, 2022 02:51
@thescientist13 thescientist13 merged commit 201b5ae into release/0.28.0 Dec 17, 2022
@thescientist13 thescientist13 deleted the feature/issue-1007-api-routes branch December 17, 2022 17:54
thescientist13 added a commit that referenced this pull request Feb 7, 2023
@thescientist13
Feature/issue 1007 api routes (#1017)
04631fc 
* API Routes

* rename ApiRoutes resource class

* add skeleton Request support to handler signature

* Response example from API route

* add local development cache busting for API routes

* refactor out dependency on path

* fix typo

* API Routes

* add local development cache busting for API routes

* refactor out dependency on path

* handle APIs for serve command

* add API development spec

* refactor API development spec to use fetch

* refactor specs

* API route serve specs

* TODOs tracking

* add API routes documentation

* restore API specs using request instead of native fetch

* remove demo code

* app exists check for API routes
thescientist13 added a commit that referenced this pull request Apr 9, 2023
@thescientist13
Feature/issue 1007 api routes (#1017)
0f2af78 
* API Routes

* rename ApiRoutes resource class

* add skeleton Request support to handler signature

* Response example from API route

* add local development cache busting for API routes

* refactor out dependency on path

* fix typo

* API Routes

* add local development cache busting for API routes

* refactor out dependency on path

* handle APIs for serve command

* add API development spec

* refactor API development spec to use fetch

* refactor specs

* API route serve specs

* TODOs tracking

* add API routes documentation

* restore API specs using request instead of native fetch

* remove demo code

* app exists check for API routes
thescientist13 added a commit that referenced this pull request Apr 9, 2023
@thescientist13
Feature/issue 1007 api routes (#1017)
b49093f 
* API Routes

* rename ApiRoutes resource class

* add skeleton Request support to handler signature

* Response example from API route

* add local development cache busting for API routes

* refactor out dependency on path

* fix typo

* API Routes

* add local development cache busting for API routes

* refactor out dependency on path

* handle APIs for serve command

* add API development spec

* refactor API development spec to use fetch

* refactor specs

* API route serve specs

* TODOs tracking

* add API routes documentation

* restore API specs using request instead of native fetch

* remove demo code

* app exists check for API routes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@thescientist13 thescientist13

Labels
CLI documentation Greenwood specific docs feature New feature or request SSR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

API Routes
1 participant
@thescientist13