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

Fix codegen-openapi build output #4509

Merged
merged 81 commits into from
Aug 30, 2024
Merged

Fix codegen-openapi build output #4509

merged 81 commits into from
Aug 30, 2024

Conversation

aryaemami59
Copy link
Contributor

@aryaemami59 aryaemami59 commented Jul 12, 2024 •
edited
Loading

#4307 Will need to be merged first.

This PR:

  • Fixes codegen-openapi build output by adding tsup.
  • Fixes the CI tests for codegen-openapi.
  • Modernizes build output by adding the exports field to provide ESM/CJS dual compatibility.
  • Uses the compact lodash.camelcase package instead of the entire lodash library in order to minimize package bundle size.
  • Updates TypeScript and Vitest to make sure the library is compatible with the latest versions of those libraries.
  • Fixes the unit tests by copying over the changes made in Fix codegen-openapi tests #4307.
  • Adds vite-tsconfig-paths to get a similar setup for unit tests as @reduxjs/toolkit.

aryaemami59 added 30 commits June 19, 2024 16:21
@aryaemami59
Add tsup
886b1c0
@aryaemami59
Remove esbuild from resolutions field
e46180d
@aryaemami59
Fix tsconfig.json
05931ea
@aryaemami59
Add tsup config
92b3cdf
@aryaemami59
Merge branch 'master' of https://github.com/reduxjs/redux-toolkit int…
45e3955 
…o tsup-codegen
@aryaemami59
Sort tsconfig.json
2259a5b
@aryaemami59
Add "sideEffects": false in package.json
d3c6886
@aryaemami59
Fix tsup.config.ts
7446675
@aryaemami59
Add createRequire calls to get require.resolve working
690506f
@aryaemami59
Update lockfile
71d8f71
@aryaemami59
Merge branch 'master' of https://github.com/reduxjs/redux-toolkit int…
a28353a 
…o tsup-codegen
@aryaemami59
Add esbuild as a dev dependency for toolkit
6c2d13b 
- This was done mostly to prevent TypeScript from using the hoisted version of `esbuild` which caused a type error since the types in version 0.21.5 (the hoisted version) don't match the types in version 0.18.20 (the version `tsup` uses as a direct dependency).
@aryaemami59
Merge branch 'master' of https://github.com/reduxjs/redux-toolkit int…
75c5375 
…o tsup-codegen
@aryaemami59
Remove external
1e50373
@aryaemami59
Run type checker with vitest
715070e
@aryaemami59
Bump oazapfts to version 6.1.0
a76c288
@aryaemami59
Fix TS errors in generate.ts
ac1f8b9
@aryaemami59
Fix unit tests in cli.test.ts
03dd1b7
@aryaemami59
Add exports field
d40e992
@aryaemami59
Use ESM cli file
3fc04b1
@aryaemami59
Fix createRequire calls to use __filename instead
da44c43
@aryaemami59
Change prepare to prepack
6408354
@aryaemami59
Fix unit tests in generateEndpoints.test.ts
3a07691
@aryaemami59
Update test-codegen.yml with steps for building and testing artifacts
317be8e
@aryaemami59
Remove unnecessary tsconfig.json file
2fa1108
@aryaemami59
Set TEST_DIST to true in CI
868df32
@aryaemami59
Fix issues in vitest.config.mts
491defd
@aryaemami59
Add tsconfig.build.json
5aca58c
@aryaemami59
Fix tsconfig.json
9973256
@aryaemami59
Add vite-tsconfig-paths
822e804
@aryaemami59 aryaemami59 mentioned this pull request Aug 27, 2024
Merged
4 tasks
aryaemami59 and others added 4 commits August 29, 2024 16:30
@aryaemami59
Merge branch 'master' of https://github.com/reduxjs/redux-toolkit int…
d781232 
…o tsup-codegen
@markerikson
Merge branch 'master' into tsup-codegen
4936532 
# Conflicts:
#	.github/workflows/test-codegen.yml
#	packages/rtk-query-codegen-openapi/package.json
#	yarn.lock
@markerikson
Tweak task names
629892c
@markerikson
Restore TEST_DIST variable
f8bcea0
@markerikson
Copy link
Collaborator

Okay, merged the test changes over and this is clean.

@aryaemami59 just to check: could you remind me why we need any of the build setup changes in the first place?

@aryaemami59
Copy link
Contributor Author

Because the current build output is broken. The reason why it's broken is because we build with moduleResolution set to Bundler but we don't use one. So our build output is ESM but the library is CJS. Hence why we get the "cannot use import statement outside of a module" error.

@markerikson
Copy link
Collaborator

Okay. Can we do this PR without the package.exports change at all? That will require a major version bump. We can do that if necessary, I just don't want to do it casually.

@aryaemami59
Copy link
Contributor Author

1 - I don't think it can be done without.
2 - Since we're changing the entire build step, I think this could be considered a breaking change even without the exports field added.

@markerikson
Copy link
Collaborator

Okay. Any idea what other codegen-related changes (PRs, issues, etc) ought to go in as well?

@aryaemami59
Copy link
Contributor Author

I'm not sure tbh. #4568 looks good.

@markerikson
Merge branch 'master' into tsup-codegen
327556b 
# Conflicts:
#	packages/rtk-query-codegen-openapi/package.json
#	yarn.lock
@markerikson markerikson merged commit a985f93 into reduxjs:master Aug 30, 2024
43 checks passed
@markerikson markerikson deleted the tsup-codegen branch August 30, 2024 22:31
@giammyisjammy giammyisjammy mentioned this pull request Sep 12, 2024
Closed
@aryaemami59 aryaemami59 linked an issue Jan 3, 2025 that may be closed by this pull request
RTKQuery Codegen execution error #3866
Closed
@aryaemami59 aryaemami59 mentioned this pull request Jan 3, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
rtkq-codegen
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RTKQuery Codegen execution error
2 participants
@aryaemami59 @markerikson