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

Support slownews.kr #305

Merged
merged 2 commits into from
Feb 18, 2019
Merged

Support slownews.kr #305

merged 2 commits into from
Feb 18, 2019

Conversation

yous
Copy link
Collaborator

@yous yous commented Feb 18, 2019

@yous
Copy link
Collaborator Author

yous commented Feb 18, 2019

#304 가 아니고 #224 인데 잘못 적었네요.

@disjukr disjukr merged commit c91839b into master Feb 18, 2019
@disjukr disjukr deleted the add-slownews branch February 18, 2019 10:41
@disjukr
Copy link
Owner

disjukr commented Feb 18, 2019

#214 이거 관련으로 갑자기 생각났는데

가능하다면 작업하실 때 fb249c2#diff-5f57ac783f7b167581e4621d66054b07R13 이런 코드를 같이 넣어주시면 좋을 것 같습니다.

readyToParse를 구현하면 필요한 내용까지만 사이트를 로드하고 바로 parse 및 reconstruct를 수행합니다.

@yous
Copy link
Collaborator Author

yous commented Feb 18, 2019

오, 이런 게 있었군요. 알겠습니다.

@yous
Copy link
Collaborator Author

yous commented Feb 18, 2019

혹시 이 기능 확인은 어떤 식으로 하시는지 알 수 있을까요? 그냥 하다보면 부정확할 수 있을 것 같아서요.

@disjukr
Copy link
Owner

disjukr commented Feb 18, 2019

@yous 구현이 잘 작동하는지 확인을 어떻게 하느냐는 말씀이시면 그냥 물떠놓고 잘 됐겠지 기도하는 겁... 이라기보다
seo를 개차반으로 하지 않는 이상 (뉴스 사이트가 그럴 리가 없죠) js로 기사 내용을 불러오지는 않을 테고 기사 전문을 html에 넣어서는 내려주겠죠.
그렇다면 html에서 기사 내용보다 뒤에 오는 것이 확실할 dom element가 문서에 존재하는지 확인하기만 하면 될 거에요.
...써놓고 보니 역시 물떠놓고 기다리는거랑 다를게 없지만 아무렴 어때요 하하

구현은 아래와 같습니다.
https://github.com/disjukr/just-news/blob/master/src/index.ts#L51-L55

근데 지금보니 저렇게 구현할게 아니고 Promise.race를 때려야 겠네요.

@yous
Copy link
Collaborator Author

yous commented Feb 18, 2019

기사 내용이랑 작성일, 기자 정보 등이 전부 HTML로 들어와서 오는 경우엔, 직접 본문 DOM element를 기다려서 바로 파싱해도 문제가 없을까요?

@disjukr
Copy link
Owner

disjukr commented Feb 18, 2019

본문 dom element의 next sibling을 기다리는게 좋을 것 같아요

@yous
Copy link
Collaborator Author

yous commented Feb 18, 2019

아, 그게 좋겠네요. 감사합니다.

@disjukr disjukr mentioned this pull request Feb 22, 2019
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