Skip to content

Fix build error #3416

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Apr 19, 2025
Merged

Fix build error #3416

merged 1 commit into from
Apr 19, 2025

Conversation

zukilover
Copy link

This PR addresses build system issues and TypeScript configuration problems in the react-google-maps-api package. The changes improve type safety and modernize the build process while maintaining compatibility with existing code.

Build System Updates

  • Updated rollup to v3.29.4 for better ESM support
  • Replaced rollup-plugin-terser with @rollup/plugin-terser for rollup v3 compatibility
  • Added type: "module" to package.json for proper ESM support

TypeScript Configuration

  • Replaced deprecated importsNotUsedAsValues with verbatimModuleSyntax
  • Added es2015 to lib array in root tsconfig.json
  • Added proper type definitions with @types/invariant
  • Fixed type imports using the new type import syntax
  • Reorganized utility exports for better type safety

Code Organization

  • Created new default-load-script.ts utility file
  • Updated import paths to use consistent module resolution
  • Removed unused imports (Loader from @googlemaps/js-api-loader)

The build process now completes successfully. There are some TypeScript warnings in test files that don't affect the build output and can be addressed separately.

@zukilover
Copy link
Author

In the latest update, here are what I've done:

  • Pin react-docgen to version 6.0.0-alpha.3 to resolve ESM compatibility issues
  • Fix documentation setup by adding browser-compatible configuration
  • Add mock Google Maps API setup for documentation environment
  • Update paths to documentation files to match correct case and location
  • Add webpack configuration for proper module resolution
  • Created mock googleMapKey.ts for documentation purposes
  • Updated styleguide.setup.js to use browser-compatible imports

@JustFly1984
Copy link
Owner

@zukilover please look into upgrade PR branch - a lot of what you are doing is already done there.

@JustFly1984 JustFly1984 changed the base branch from develop to upgrade March 18, 2025 09:48
@JustFly1984
Copy link
Owner

not sure if adding type: module in package.json is ok for a package, cos it will ruin the day for those who is on old frameworks.

@JustFly1984
Copy link
Owner

@zukilover Sorry, I have re-target your PR to upgrade branch, hence merge conflicts. Can you resolve it? if not, I can help out.

@JustFly1984
Copy link
Owner

@zukilover Awesome work, could not have done it better myself.

@zukilover
Copy link
Author

@zukilover Sorry, I have re-target your PR to upgrade branch, hence merge conflicts. Can you resolve it? if not, I can help out.

Sure, I'll work on resolving the conflicts.

@zukilover
Copy link
Author

not sure if adding type: module in package.json is ok for a package, cos it will ruin the day for those who is on old frameworks.

Yeah this could be problematic, I'll see if I can keep it as it was.

@JustFly1984
Copy link
Owner

@zukilover
type module is fine on the monorepo level in root package.json, so maybe correct way would be to extract docs into separate package, if it requires to add type: module.
PS - I've moved to pnpm instead of yarn in upgrade branch.

@JustFly1984
Copy link
Owner

@zukilover are you going to continue on this PR?

@zukilover
Copy link
Author

zukilover commented Apr 18, 2025

@JustFly1984 Hi, sorry for the late update. After reviewing the upgrade branch, I can see what you mentioned about a lot of the work already being there. What I did from the latest rebase was to fix the build from the styleguidist config file — this works for me now. Let me know if this looks good to you.

I switched to CommonJS (require) locally in the config and updated the Webpack setup to make it compatible.

@JustFly1984 JustFly1984 merged commit b69f847 into JustFly1984:upgrade Apr 19, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants