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 all commits
Commits
File filter

Filter by extension

Filter by extension

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

Fix Proto loading for Vercel/Next.js.
33 changes: 26 additions & 7 deletions packages/firestore/rollup.config.js
Original file line number Diff line number Diff line change
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,34 @@ 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 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.

// Although this is a cjs Node build and shouldn't require the browser option,
// Vercel apps using this break on deployment, but work in local development.
// See https://github.com/firebase/firebase-js-sdk/issues/5823
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`;
// Added a check for Jest (see issue 5687 linked above)
// See https://jestjs.io/docs/environment-variables - apparently
// these are not always both set.
const JEST_CHECK =
`typeof process !== 'undefined' && process.env !== undefined` +
` && (process.env.JEST_WORKER_ID !== undefined || ` +
`process.env.NODE_ENV === 'test')`;
// Copied from rollup output
return (
`((typeof document === 'undefined' || (${JEST_CHECK})) ?` +
` 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 +143,7 @@ const allBuilds = [
plugins: [
...util.es2017ToEs5Plugins(/* mangled= */ false),
replace(generateBuildTargetReplaceConfig('cjs', 2017)),
importMetaUrlPolyfillPlugin()
importMetaUrlPolyfillPlugin(basename(pkg.main))
],
external: util.resolveNodeExterns,
treeshake: {
Expand Down