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

Put firebase/* entry point ESM files in their own subdirs with "type: module" package.json #6826

Merged
merged 2 commits into from
Dec 1, 2022

Conversation

hsubox76
Copy link
Contributor

See #6747

More build tooling these days is requiring all ESM bundles conform to Node ESM rules, even if the bundles are mainly used in the browser (for example, many test frameworks run browser tests in Node and require all bundles to meet Node ESM rules when the tests are run). This may also affect SSR/SSG.

The rules are essentially that the extension must be .mjs OR the nearest package.json file must have a line that says type: "module".

We avoid putting an mjs extension on browser bundles because some older build tools and environments don't know how to handle that extension. Instead, in the @firebase/* packages, we have put ESM files in their own subdirectory in dist/ (dist/esm/) and used a rollup plugin put a one-line package.json file in there with them that says "type: module". This build step had not been applied to the firebase/* packages (the entry point packages that just redirect to the equivalent @firebase/* package).

It's possible that some tooling is happy enough with the end package (@firebase/*) having this setup (the way it is now), while other tooling requires the Node ESM indicators to be there at every step of resolution (explaining issues still being reported by some users).

I've added this step to the entry point firebase/* packages in this PR hoping this will fix it for those users.

@changeset-bot
Copy link

changeset-bot bot commented Nov 22, 2022

🦋 Changeset detected

Latest commit: b4bec51

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
firebase Patch

Not sure what this means? Click here to learn what changesets are.

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 22, 2022

Size Report 1

Affected Products

  • @firebase/firestore

    TypeBase (ab3f16c)Merge (b7e8b51)Diff
    browser266 kB272 kB+6.37 kB (+2.4%)
    esm5332 kB339 kB+7.38 kB (+2.2%)
    main531 kB545 kB+13.7 kB (+2.6%)
    module266 kB272 kB+6.37 kB (+2.4%)
    react-native266 kB272 kB+6.37 kB (+2.4%)
  • @firebase/firestore-lite

    TypeBase (ab3f16c)Merge (b7e8b51)Diff
    browser82.4 kB85.9 kB+3.45 kB (+4.2%)
    esm599.5 kB104 kB+4.25 kB (+4.3%)
    main139 kB146 kB+6.95 kB (+5.0%)
    module82.4 kB85.9 kB+3.45 kB (+4.2%)
    react-native82.6 kB86.1 kB+3.45 kB (+4.2%)
  • bundle

    12 size changes

    TypeBase (ab3f16c)Merge (b7e8b51)Diff
    firestore (Persistence)276 kB281 kB+4.24 kB (+1.5%)
    firestore (Query Cursors)213 kB220 kB+6.57 kB (+3.1%)
    firestore (Query)214 kB217 kB+3.25 kB (+1.5%)
    firestore (Read data once)202 kB204 kB+1.90 kB (+0.9%)
    firestore (Realtime updates)205 kB207 kB+1.90 kB (+0.9%)
    firestore (Transaction)186 kB188 kB+1.69 kB (+0.9%)
    firestore (Write data)186 kB188 kB+1.69 kB (+0.9%)
    firestore-lite (Query Cursors)71.5 kB81.0 kB+9.46 kB (+13.2%)
    firestore-lite (Query)74.7 kB77.2 kB+2.42 kB (+3.2%)
    firestore-lite (Read data once)59.1 kB59.1 kB-3 B (-0.0%)
    firestore-lite (Transaction)83.7 kB83.7 kB-3 B (-0.0%)
    firestore-lite (Write data)68.9 kB68.9 kB-3 B (-0.0%)

  • firebase

    TypeBase (ab3f16c)Merge (b7e8b51)Diff
    firebase-compat.js740 kB746 kB+5.64 kB (+0.8%)
    firebase-firestore-compat.js314 kB319 kB+5.64 kB (+1.8%)
    firebase-firestore-lite.js89.1 kB92.6 kB+3.41 kB (+3.8%)
    firebase-firestore.js314 kB321 kB+6.32 kB (+2.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/KADFgxq7xy.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 22, 2022

Size Analysis Report 1

This report is too large (759,964 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/4Xsbm909zo.html

@hsubox76 hsubox76 marked this pull request as ready for review November 30, 2022 18:45
Copy link
Contributor

@dwyfrequency dwyfrequency left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hsubox76 hsubox76 merged commit 7e237cd into master Dec 1, 2022
@hsubox76 hsubox76 deleted the ch-esm-entry branch December 1, 2022 17:11
@google-oss-bot google-oss-bot mentioned this pull request Dec 6, 2022
levino pushed a commit to levino/firebase-js-sdk that referenced this pull request Dec 10, 2022
@firebase firebase locked and limited conversation to collaborators Jan 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants