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: support modern.js data action #4736

Merged
merged 6 commits into from
Oct 8, 2023

Conversation

yimingjfe
Copy link
Member

@yimingjfe yimingjfe commented Sep 27, 2023

Summary

This PR implements the RFC #4755.

🤖 Generated by Copilot at d28833c

This pull request adds support for data loader actions, which are optional files that define custom logic for fetching data in the @modern-js/runtime module. It refactors and updates the CLI, the runtime, and the app-tools modules to handle both loader and action requests, and to generate the appropriate code and types. It also adds and fixes some integration tests for the data loader feature, using different modes and scenarios.

Details

🤖 Generated by Copilot at d28833c

  • Add support for data loader actions in CLI, runtime, and app-tools modules
  • Create a new function createActionRequest that creates a request handler for data loader actions (link)
  • Modify the generateClient function to take an object with inline, action, and routeId properties and generate client code for data loader requests accordingly (link, link, link)
  • Modify the type Context to add more properties for data loader requests and parse them from the resourceQuery (link, link)
  • Modify the call to generateClient to pass the inline, action, and routeId properties from the options object (link)
  • Rename the function createLoaderRequest to createRequest and modify it to handle both loader and action requests (link, link, link, link, link, link, link, link)
  • Add a new constant ACTION_EXPORT_NAME that represents the name of the action export from the data loader files (link)
  • Modify the walk function to check if a data loader file has an action export and set the route.action property accordingly (link, link, link, link, link, link, link)
  • Modify the generateRoutes and generateLoaders functions to include the action property in the route objects and the loadersMap object, and to use the loaderId, route, and action properties to generate the client code for data loader requests (link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Add a new function hasAction that checks if a data loader file has an action export or not (link, link)
  • Add a new test file data.test.ts that tests the hasLoader and hasAction functions (link)
  • Add a new data file $.data.ts that exports a loader and an action function as named exports (link)
  • Add a new file utils.ts that exports a constant modernTestActionName that represents the name of the action used in the integration tests (link)
  • Add a new data file page.data.ts in the user/[id] directory that exports a loader and an action function as named exports, and use them in the page.tsx file in the same directory (link, link, link)
  • Add a new data file page.data.ts in the user/[id]/profile directory that exports a loader and an action function as named exports, and use them in the page.tsx file in the same directory (link, link, link)
  • Add a new data file page.data.ts in the user/profile directory that exports a loader function as a named export (link)
  • Add two new functions supportActionInCSR and supportActionInSSR that test the data loader actions in CSR and SSR modes respectively, and use them in the test cases 'support action in CSR' and 'support action in SSR' in the index.test.ts file (link, link, link, link)
  • Modify the supportNoLayoutDir function to expect the correct text for the user/[id] route (link)
  • Remove the commented out code that creates a new page (link)
  • Remove the skip annotations from the test cases 'support handle loader error' and 'support loader for ssr and csr', as they are fixed by the pull request (link, link, link, link, link)
  • Delete the unused file page.loader.ts in the user/profile directory (link)

Related Issue

Checklist

  • I have added changeset via pnpm run change.
  • I have updated the documentation.
  • I have added tests to cover my changes.

@changeset-bot
Copy link

changeset-bot bot commented Sep 27, 2023

⚠️ No Changeset found

Latest commit: c3aaec5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (99693f0) 57.59% compared to head (c3aaec5) 50.52%.
Report is 1233 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4736      +/-   ##
==========================================
- Coverage   57.59%   50.52%   -7.07%     
==========================================
  Files         672      691      +19     
  Lines       17794    19203    +1409     
  Branches     3876     4431     +555     
==========================================
- Hits        10249     9703     -546     
- Misses       6934     8746    +1812     
- Partials      611      754     +143     

see 509 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@GiveMe-A-Name GiveMe-A-Name changed the title feat: support edenx data action feat: support modern.js data action Sep 28, 2023
@chenjiahan chenjiahan mentioned this pull request Sep 28, 2023
@yimingjfe yimingjfe enabled auto-merge (squash) October 8, 2023 07:24
@yimingjfe yimingjfe merged commit d54a7be into web-infra-dev:main Oct 8, 2023
9 checks passed
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.

2 participants