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

Change conditional check for window to use typeof window #4970

Merged
merged 2 commits into from
Jun 23, 2020

Conversation

META-DREAMER
Copy link
Contributor

This allows the code to run safely in a non-DOM environment

META-DREAMER added a commit to sourcecred/sourcecred that referenced this pull request Jun 22, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
Copy link
Contributor

@djhi djhi left a comment

Choose a reason for hiding this comment

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

Thanks!

@djhi djhi added this to the 3.6.2 milestone Jun 23, 2020
@djhi djhi merged commit 626412c into marmelab:master Jun 23, 2020
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jun 27, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jun 27, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jun 27, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
META-DREAMER added a commit to sourcecred/sourcecred that referenced this pull request Jun 28, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jun 28, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 7, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 11, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 11, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 11, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 11, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 11, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 11, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 11, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
META-DREAMER added a commit to sourcecred/sourcecred that referenced this pull request Jul 12, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 13, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 13, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 14, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 14, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 14, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 14, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
META-DREAMER added a commit to sourcecred/sourcecred that referenced this pull request Jul 14, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
META-DREAMER added a commit to sourcecred/sourcecred that referenced this pull request Jul 14, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
META-DREAMER added a commit to sourcecred/sourcecred that referenced this pull request Jul 14, 2020
* Setup React Admin frontend

This configures ReactAdmin to render as the frontend with a simple "fake data provider".
SSR issues need to be addressed before the UI will be visible

TestPlan: Ensure the frontend builds when running yarn start

* Fix failing react-admin build

This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors

* Update history to compatible version

Updates the history package to a version compatible with the React Router v4

Test Plan: Run yarn start --instance <instance location> and ensure the
 React Admin UI renders and navigation works

* Fix flow errors importing from history package

v4 of the history package updated the import format, updated our usage to match.
More info: https://github.com/ReactTraining/history/blob/845d690c5576c7f55ecbe14babe0092e8e5bc2bb/CHANGES.md#v400-0

TestPlan: Ensure flow check passes

* Add flow types for History and React-Router

Better type safety is always nice to have :)

TestPlan: Ensure versions for typings match installed versions of history and react-router

* remove withAssets and createRelativeHistory

withAssets is no longer needed since we have the Docusaurus site to fulfill those purposes and
our homepage is no longer being generated by this UI.

RelativeHistroy is no longer needed with Router V5 because this version
of the router accepts a `basename` parameter

TestPlan: Ensure that nothing is still using functionality from these two modules

* Update Link component to react-router v5

React router v5 has moved its Link component into a separate package: react-router-dom. This PR
just updates the place we import the Link component for our own internal wrapper around it.

TestPlan: Ensure that our Link component is still functional

* Remove react-admin patches

React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors

Co-authored-by: Kevin Siegler <kevinsiegler54@gmail.com>
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 20, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 20, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 20, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 20, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 20, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 20, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 20, 2020
This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors
topocount pushed a commit to sourcecred/sourcecred that referenced this pull request Jul 20, 2020
React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors
youngkidwarrior pushed a commit to youngkidwarrior/sourcecred that referenced this pull request Aug 20, 2020
* Setup React Admin frontend

This configures ReactAdmin to render as the frontend with a simple "fake data provider".
SSR issues need to be addressed before the UI will be visible

TestPlan: Ensure the frontend builds when running yarn start

* Fix failing react-admin build

This mocks out the testing-library modules which were causing issues in the frontend build and adds temporary
patches on postinstall for the other problematic react-admin libraries. We can remove the patches once they merge
the PR that fixes this upstream: marmelab/react-admin#4970

Test Plan: Run yarn start --instance <instance location> to ensure the frontend builds without errors

* Update history to compatible version

Updates the history package to a version compatible with the React Router v4

Test Plan: Run yarn start --instance <instance location> and ensure the
 React Admin UI renders and navigation works

* Fix flow errors importing from history package

v4 of the history package updated the import format, updated our usage to match.
More info: https://github.com/ReactTraining/history/blob/845d690c5576c7f55ecbe14babe0092e8e5bc2bb/CHANGES.md#v400-0

TestPlan: Ensure flow check passes

* Add flow types for History and React-Router

Better type safety is always nice to have :)

TestPlan: Ensure versions for typings match installed versions of history and react-router

* remove withAssets and createRelativeHistory

withAssets is no longer needed since we have the Docusaurus site to fulfill those purposes and
our homepage is no longer being generated by this UI.

RelativeHistroy is no longer needed with Router V5 because this version
of the router accepts a `basename` parameter

TestPlan: Ensure that nothing is still using functionality from these two modules

* Update Link component to react-router v5

React router v5 has moved its Link component into a separate package: react-router-dom. This PR
just updates the place we import the Link component for our own internal wrapper around it.

TestPlan: Ensure that our Link component is still functional

* Remove react-admin patches

React admin merged my PR that fixes the issues with window (marmelab/react-admin#4970),
so we no longer need these patches since the issue is addressed upstream now.

TestPlan: Ensure that the site builds and runs without "window is not defined" errors

Co-authored-by: Kevin Siegler <kevinsiegler54@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants