The Wayback Machine - https://web.archive.org/web/20250605193840/https://github.com/ionic-team/ionic-framework/pull/21209
Skip to content

[React] ssr ReferenceError: document is not defined #21209

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

Closed
wants to merge 49 commits into from

Conversation

wis
Copy link

@wis wis commented May 5, 2020

When using react output generated components do not work in ssr (like Next.js) because generated component try to use document without safety checking if document exists.

default function parameters values of built in globals is not a good pattern to use when SSR support is a goal of the project, so can you https://www.youtube.com/watch?v=4gYZsCm5blA :)

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

ReferenceError: document is not defined when SSRing

Issue Number: N/A

What is the new behavior?

no ReferenceError: document is not defined

Does this introduce a breaking change?

  • Yes
  • No

Other information

Wis and others added 30 commits May 5, 2020 05:03
When using react output generated components do not work in ssr (like Next.js) because generated component try to use document without safety checking if document exists. 

default function parameters values of built in globals is not a good pattern to use when SSR support is a goal of the project, so can you https://www.youtube.com/watch?v=4gYZsCm5blA :)
…ionic-team#21283)

- improves the documentation on customizing scoped overlays using cssClass and/or CSS variables
- includes a section in the Angular usage with information on where the CSS needs to be styled (globally) in order to work for an overlay
…onic-team#21170)

* fix(input): check for tabindex and pass it properly to native input

references ionic-team#17515

* style(input): fix lint error

* test(input): update test for more use cases (inside item)

* fix(item): adds delegatesFocus to shadow

* style(input): add comment block on what the code does
manucorporat and others added 14 commits May 22, 2020 09:23
* chore: internal import updates to improve bundling

- Rename keyboard.ts so it has a good filename after custom element bundling
- Import util fns directly instead of from top level index
- Do not export with *

* chore(angular): bump ng-packagr

Co-authored-by: Mike Hartington <[email protected]>
inherits alignment in inner item, sets item alignment to center

fixes ionic-team#18703
@liamdebeasi liamdebeasi requested a review from elylucas June 2, 2020 13:47
@liamdebeasi
Copy link
Contributor

Thanks for the PR! Can you fix the failing test?

Wis added 3 commits June 2, 2020 20:34
in node.js document is undefined so "Expression is always false." tslint error is false.
@wis
Copy link
Author

wis commented Jun 2, 2020

I fixed all the failing tests that I could, only one is left test-core-clean-build I don't know what it is for, it seems to be a git diff command that exits with code 0 and makes test fail if it has output.

@liamdebeasi
Copy link
Contributor

Try syncing your branch with the latest master and pushing those changes. We recently updated Swiper which is probably why you are getting that failure.

@wis
Copy link
Author

wis commented Jun 2, 2020

done, all tests pass now.
edit: I think I synced master to my branch instead of vice versa/as-you-suggested, that is not an issue, right?

@liamdebeasi
Copy link
Contributor

Hmm since you forked Ionic, it looks like you will need to a) sync your fork's master branch with our master branch and b) sync your PR branch with your fork's master.

This StackOverflow answer gives a pretty good walkthrough of how to do it: https://stackoverflow.com/questions/7244321/how-do-i-update-a-github-forked-repository

Or if it's easier, just refork from Ionic and copy your changes over so you have the latest changes in Ionic.

@brandyscarney brandyscarney added the package: react @ionic/react package label Oct 1, 2020
@liamdebeasi
Copy link
Contributor

Apologies for the delay. It looks like this issue was resolved when we shipped our @ionic/react-router enhancements in v5.3.0: https://github.com/ionic-team/ionic-framework/blob/master/packages/react/src/components/utils/attachProps.ts#L70

I am going to close this PR, but feel free to open an issue if you run into any other problems with SSR. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: react @ionic/react package
Projects
None yet
Development

Successfully merging this pull request may close these issues.