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

fix: rollup/typescript configuration #835

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

usefulthink
Copy link
Contributor

@usefulthink usefulthink commented Feb 23, 2024

This is a full refactoring of the rollup configuration to speed up builds, simplify the configuration, prevent warnings and fix an error with the generated source maps.

In detail:

  • refactor: Combine the three non-esm outputs into a single configuration.

  • refactor: add {babelHelpers:'bundled'} to configuration. This doesn't change the behavior (since it's the default if unspecified), but it prevents the corresponding warning from showing up.

  • feat: Enabled source-maps for all build outputs.

  • fix: Use a separate TypeScript configuration (tsconfig.build.json) during build. This configuration only compiles the required typescript files and doesn't reproduce the directory structure in the dist-folder (see also the change in package.json). It also now generates source maps correctly (Bug: Failed to parse source map #834).

  • chore: Moved fast-deep-equal from dependencies to devDependencies. It is a very small function, and it has always been bundled in all of our outputs, so it's not actually required at runtime. This will be changed in the next major release.

fixes #834

This is a full refactoring of the rollup configuration to speed up builds, simplify the configuration prevent warnings and fix an error with the generated source maps.

In detail:
 - refactor: Combine the three non-esm outputs into a single configuration.

 - feat: Enabled source-maps for all build outputs.

 - fix: Use a separate TypeScript configuration (`tsconfig.build.json`) during build. This configuration only compiles the required typescript files and doesn't reproduce the directory structure in the dist-folder (see also the change in package.json). It also now generates source maps correctly (googlemaps#834).

 - chore: Moved `fast-deep-equal` from dependencies to devDependencies. It is a very small function, and it has always been bundled in all of our outputs, so it's not actually required at runtime. This will be changed in the next major release.

 fixes googlemaps#834
@usefulthink
Copy link
Contributor Author

usefulthink commented Feb 23, 2024

Additional testing:

Ran publint over the new package. Result:

@googlemaps/js-api-loader lint results:
Suggestions:
1. pkg.module is used to output ESM, but pkg.exports is not defined. As NodeJS doesn't read pkg.module, the ESM output may be skipped. Consider adding pkg.exports to export the ESM output. pkg.module can usually be removed alongside too. (This will be a breaking change)

So no errors or warnings, migration to using only exports is something for the next major release.

Install tests in multiple setups (always installing this package via npm pack ../path/to/js-api-loader and npm install googlemaps-js-api-loader-x.xx.xx.tgz) and confirmed

  • no build-errors or warnings
  • types are available
  • package works as expected
  • debugging in chrome works with sourcemaps

Test setups:

  • create-react-app (webpack / TS)
  • next.js (babel / TS)
  • vite (vite / TS)
  • loading UMD with script-tag (no bundler / JS)
  • loading IIFE (min/dev) with script-tag
  • loading ESM with native import

@usefulthink usefulthink added the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Failed to parse source map
1 participant