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

Great plugin but won't work with server-side rendering #5

Closed
lucidlive opened this issue Jan 31, 2017 · 11 comments
Closed

Great plugin but won't work with server-side rendering #5

lucidlive opened this issue Jan 31, 2017 · 11 comments
Labels

Comments

@lucidlive
Copy link

I get the error:
Error: Cannot find module 'tween'

@willhoag
Copy link
Owner

willhoag commented Feb 2, 2017

Looks like this is an issue with the scroll-to dependency.

Tracking for this is here:
component/scroll-to#9

Could be worth replacing the dependency as this issue hasn't been addressed for a while now.

@willhoag willhoag added the bug label Feb 2, 2017
@Swivelgames
Copy link

poke

@monteiz
Copy link

monteiz commented Jul 19, 2017

If you use React, move your function declaration in the componentDidMount method:

let scrollToElement; 

export default class MyComponent extends React.Component {
	...
	componentDidMount() {
		scrollToElement = require('scroll-to-element');
	}
	...
}

@Swivelgames
Copy link

Swivelgames commented Aug 1, 2017

@monteiz componentDidMount executes on server-side. Additionally, people that conform to certain standards (like Airbnb's .eslintrc) cannot use require, and some others throw errors if require is used anywhere except for the top of the script (similar to import statements).

Is there another, more universal solution here?

@monteiz
Copy link

monteiz commented Aug 2, 2017

@Swivelgames you should first understand how React works: componentDidMount does not execute on server side, but only on the client, once the component has been rendered. There are plenty of articles and tutorials stating that, and if you do not trust them, go and test it yourself.

About my attempt to help others here: while I agree that this is far from being an elegant solution, apparently it is the only one that works so far. So feel free to use it, or if you have a better solution to share with us, post it.

Regarding "Airbnb standards"...are you really preventing yourself from using a solution because Airbnb said you should not use it? Did I understand correctly?

@Swivelgames
Copy link

@monteiz It sounds like you're a little chagrined. My response was not meant to be insulting or embarrassing, and certainly not to solicit an abundant amount of your energy getting worked up on writing such a response. I merely wanted to point out that your solution was not the ideal solution. I recommend against taking things personally in these scenarios, as it tends to make a place that's usually inviting to friendly discussions about workable solutions and just makes Github a sour place to be. We're all here to do what we love, work on projects, and find the best solutions 😃

My question for a more workable and universal solution was directed towards the maintainer or a knowledgeable, potential contributor, not directly to you. It was asked in the hopes that someone would be able to contribute a proper fix for this. I've put it on my list of issues to look into myself with a potential PR, but I can't always prioritize every issue on every package I use above all the others.

You're welcome to submit a pull request yourself with a usable universal solution and I'm sure @willhoag would be happy to use it. 👍

In regards to "Airbnb standards," you seem to be uninformed. Many efficient teams involved in notable projects within notable companies want to prevent the common mistake of mixing coding styles and not monitoring for best practices which makes most projects difficult to maintain and unworkable in the long term, forcing companies to spend extra money rewriting softwares that accumulate too much tech debt too quickly

To prevent this, they adopt a set of standards and policies that they conform to in order to improve maintainability and readability across teams and team members, technology future-proofing, reduce potential tech debt, and improve efficiency.

This is much different than the traditional sense of "corporate red-tape and policies," although it can manifest itself that way at some places stuck in their old ways (similar to the way that some teams refuse to adopt any types of standards). When these policies and standards being to hinder the aforementioned, they're set aside an exceptions are made. But it doesn't mean it's always an acceptable solution to code outside of standards just because it might be "too hard" to think up a better solution. 😄

Specifically, Airbnb's .eslintrc is employed in many cases to enforce these standards and prevent the occasional "Oops" (typically throwing an error in the editor, preventing check-ins, and other things available when using a ESLint). However, in the more general sense, require() is more and more becoming less of a best practice, and certainly not a good practice when employed away from the top of scripts. This is much different than how it was originally used when commonjs and others were originally introduced, but it certainly increases the maintainability for sure.

If you'd like, I may be able to get a hold of some common standards that you can review for your own personal use on some of your projects. But, as I mentioned before, Airbnb has become an awesome resource for some good, workable standards and packages that don't get in the way of efficiently developing within most teams. It also prevents bad practices in code that may turn into tech debt later on, like continuing to use require() in ES6 (which is usually used in an attempt to future-proof code 😉 ).

@monteiz
Copy link

monteiz commented Aug 28, 2017

@Swivelgames your problem is that you did not catch the joke.

And again, your argument is completely wrong: you are confused between standard and code convention 😂. I tell you the difference:

  • you should follow a standard
  • you are free to use a code convention

Airbnb's is not a standard. It is their code convention, that of course you should follow if working for them, but not otherwise 😂

So buddy, if you are afraid to use my workaround, or if you still believe that "componentDidMount executes on server-side" 😂 you should simply do what @willhoag already said, because he already gave you the solution: replace the dependency of the tween module!

If you had time to write such a long comment, I am sure you have time to fix this bug too, otherwise someone here might think that time isn't the only problem you are having with that!

@willhoag
Copy link
Owner

willhoag commented Aug 28, 2017

Please keep conversations constructive please. I'm placing a temporary (1d) ban on all public repo activity as a "cool-down" period. Thank you for understanding. @Swivelgames @monteiz

@coreyward
Copy link

For what it's worth, this issue with scroll-to prevent usage of this lib with Gatsby websites. I don't believe it's possible to combine require and import syntax in Webpack anymore either, so the workaround doesn't really work.

  1. This error is caused by scroll-to using browser aliases in package.json (easy to drop)
  2. The scroll-to code is super short
  3. scroll-to has been abandoned

Given this, would it be possible to go ahead and maintain a copy of the relevant scroll-to code inside of this library rather than continuing to rely on scroll-to?

@willhoag
Copy link
Owner

Yep, I like that idea. Should be easy enough to do. Will have a look today. Thanks. 👍

@willhoag
Copy link
Owner

Ok, published. 2.0.1 👍

Did a quick eye test with examples/index.html. Should write real tests for this thing at some point.

Closing for now. Thanks.

  • Will

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants