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

Basic support for server-side-rendering #969

Merged
merged 1 commit into from Aug 1, 2017
Merged

Conversation

nfriedly
Copy link
Contributor

@nfriedly nfriedly commented Aug 1, 2017

This checks if window exists before checking window.navigator when creating the default config.

Without this check, Flatpickr cannot even be loaded in Node.js (for e.g. server-side-rendering of the react component).

This has a side-effect of disabling animation in Node.js :P

I also added a very simple test that just runs require('require('./src/flatpickr.js') in Node.js. A more comprehensive test might be to build the dist folder and then require dist/flatpickr.js, I can switch it if you'd prefer. Note that this test must run outside of Jest, because Jest automatically pollyfills window, which would make it pass even when it shouldn't.

This fixes the root cause of the following issues:

This checks if `window` exists before checking `window.navigator` in the default config.
Without this check, Flatpickr cannot be loaded in Node.js (for e.g. server-side-rendering the react component).

This has a side-effect of disabling animation in non-browser contexts :P

I also added a very simple test that just require's the Flatpickr.js source file in node.js. A more comprehensive test would build the dist folder and then require that, but this is probably good enough for now. Note that this test must run outside of Jest, because Jest automatically pollyfills window, which would make it pass even when it shouldn't.

This fixes the root cause of the following issues:
* haoxins/react-flatpickr#9
* https://github.com/carbon-design-system/carbon-components-react/issues/76
@coveralls
Copy link

Coverage Status

Coverage remained the same at 83.638% when pulling c778ca7 on nfriedly:ssr into b08223a on chmln:master.

@chmln chmln merged commit fa950ff into flatpickr:master Aug 1, 2017
@chmln
Copy link
Member

chmln commented Aug 1, 2017

Looks great, thank you for the effort! 👍 👍

@nfriedly
Copy link
Contributor Author

nfriedly commented Aug 1, 2017

My pleasure, thanks for the quick response!

Can you let me know when it's released so that I can send PRs to update the other two projects?

@chmln
Copy link
Member

chmln commented Aug 1, 2017

@nfriedly Absolutely

@tw15egan
Copy link
Contributor

tw15egan commented Aug 7, 2017

Any chance a release could be published so we can resolve this issue? carbon-design-system/carbon-components-react#150

@chmln
Copy link
Member

chmln commented Aug 8, 2017

@tw15egan v3.0.7 is out

@tw15egan
Copy link
Contributor

tw15egan commented Aug 8, 2017

@chmln Thanks so much!

nfriedly added a commit to nfriedly/react-flatpickr that referenced this pull request Aug 8, 2017
haoxins pushed a commit to haoxins/react-flatpickr that referenced this pull request Aug 9, 2017
rajr5 pushed a commit to rajr5/react-flatpickr that referenced this pull request Jun 23, 2023
slava187 pushed a commit to slava187/react-flatpickr that referenced this pull request Nov 21, 2023
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.

None yet

4 participants