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 Vercel build #5830

Merged
merged 6 commits into from Jan 10, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/eight-cows-push.md
@@ -0,0 +1,5 @@
---
"@firebase/firestore": patch
---

Fix Proto loading for Vercel/Next.js.
18 changes: 12 additions & 6 deletions packages/firestore/rollup.config.js
Expand Up @@ -23,6 +23,7 @@ import replace from 'rollup-plugin-replace';
import { terser } from 'rollup-plugin-terser';
import typescriptPlugin from 'rollup-plugin-typescript2';
import tmp from 'tmp';
import { basename } from 'path';
import typescript from 'typescript';

import { generateBuildTargetReplaceConfig } from '../../scripts/build/rollup_replace_build_target';
Expand All @@ -31,16 +32,21 @@ import pkg from './package.json';

const util = require('./rollup.shared');

// Customize how import.meta.url is polyfilled in cjs nodejs build. We use it to be able to use require() in esm.
// It only generates the nodejs version of the polyfill, as opposed to the default polyfill which
// supports both browser and nodejs. The browser support is unnecessary and doesn't work well with Jest. See https://github.com/firebase/firebase-js-sdk/issues/5687
function importMetaUrlPolyfillPlugin() {
// Customize how import.meta.url is polyfilled in cjs nodejs build. We use it to
// be able to use require() in esm. It only generates the nodejs version of the
// polyfill, as opposed to the default polyfill which supports both browser and
// nodejs. The browser support is unnecessary and doesn't work well with Jest.
// See https://github.com/firebase/firebase-js-sdk/issues/5687
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-wrapped this comment

Copy link
Contributor

Choose a reason for hiding this comment

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

If you check the link, the ternary was originally removed because the document check was coming up true for Jest which caused it to take the second option which broke Jest somehow, so I think this will break Jest again. I am not sure why Vercel is using the Node bundle in what seems to be a non-Node environment but I'll try to look into that right away.

function importMetaUrlPolyfillPlugin(filename) {
return {
name: 'import-meta-url-current-module',
resolveImportMeta(property, { moduleId }) {
if (property === 'url') {
// copied from rollup output
return `new (require('url').URL)('file:' + __filename).href`;
return "(typeof document === 'undefined' ? new (require('url').URL)"
+ "('file:' + __filename).href : (document.currentScript && "
+ `document.currentScript.src || new URL('${filename}', `
+ "document.baseURI).href))";
}
return null;
}
Expand Down Expand Up @@ -124,7 +130,7 @@ const allBuilds = [
plugins: [
...util.es2017ToEs5Plugins(/* mangled= */ false),
replace(generateBuildTargetReplaceConfig('cjs', 2017)),
importMetaUrlPolyfillPlugin()
importMetaUrlPolyfillPlugin(basename(pkg.main))
],
external: util.resolveNodeExterns,
treeshake: {
Expand Down