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

Add CJS build for messaging/sw #5884

Merged
merged 2 commits into from Jan 13, 2022
Merged

Add CJS build for messaging/sw #5884

merged 2 commits into from Jan 13, 2022

Conversation

hsubox76
Copy link
Contributor

@hsubox76 hsubox76 commented Jan 13, 2022

Although the messaging SW shouldn't technically have a Node version as SW's don't run in Node, some SSR applications generate pages in Node to be later loaded in the browser, and these builds break when they can't find a CJS version. This can also be fixed by adding an ESM Node version, but that can't currently be done until we update the idb dependency (see #5839). Adding a CJS build for now.

See #5854

@hsubox76 hsubox76 requested a review from zwu52 as a code owner Jan 13, 2022
@changeset-bot
Copy link

@changeset-bot changeset-bot bot commented Jan 13, 2022

🦋 Changeset detected

Latest commit: b021aba

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

This PR includes changesets to release 3 packages
Name Type
@firebase/messaging Patch
firebase Patch
@firebase/messaging-compat 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 google-oss-bot commented Jan 13, 2022

@google-oss-bot
Copy link
Contributor

@google-oss-bot google-oss-bot commented Jan 13, 2022

@zwu52
Copy link
Member

@zwu52 zwu52 commented Jan 13, 2022

LG. 1 q: This can also be fixed by adding an ESM Node version, but that can't currently be done until we update the idb dependency: does this mean that this change wants to be reverted after the idb upgrade ? If so, might be worth to add a TODO to remove

zwu52
zwu52 approved these changes Jan 13, 2022
@hsubox76 hsubox76 requested a review from egilmorez as a code owner Jan 13, 2022
@hsubox76 hsubox76 merged commit 88d43ec into master Jan 13, 2022
17 of 18 checks passed
@hsubox76 hsubox76 deleted the ch-msg-builds branch Jan 13, 2022
@google-oss-bot google-oss-bot mentioned this pull request Jan 13, 2022
@firebase firebase locked and limited conversation to collaborators Feb 13, 2022
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.

None yet

3 participants