Skip to content

Conversation

@chestozo
Copy link
Member

@chestozo chestozo commented May 3, 2014

Изменения API:

  • ns.tmpl теперь возвращает строку. До сих пор он возвращал DOM ноду, но у нас нет DOM-а на сервере (он нам и не нужен)
  • ns.update теперь поддерживает options.syncOnly (выполнить update только для синхронных видов) и options.renderOnly (результатом update-а должна стать HTML строка)

Особенности работы:

Пример:
допилил своё демо приложение https://github.com/chestozo/noscript-demo/

git clone git@github.com:chestozo/noscript-demo.git
cd noscript-demo
npm install
node server-data.js &
node server.js
# -> http://localhost:2014/
# Тут будет отдаваться html, сгенерённый на сервере.
# При этом одна из моделей грузится по http на стороне nodejs.

А ещё я хочу сказать вот что:

  • DOM не потребовался, потому что доработал ns.update
  • вообще основная работа по прикручиванию ложится на плечи разработчиков приложения, в noscript почти ничего упростить нельзя
  • нужно внимательно писать yate external функции. Там нельзя использовать глобальные объекты, которые есть в браузере (window, document, location). И вообще все глобальные объекты должны быть доступны в nodejs и их нужно передавать в yate, как тут https://github.com/chestozo/noscript-demo/blob/master/server.js#L24
  • чтобы это заработало, нужно, чтобы @pasaran замёржил Возможность указать глобальные объекты во время шаблонизации в nodejs pasaran/yate#226
  • надо ещё научиться как-то вырезать корневую ноду, которая прилетает из yate <div id="ns-root"> ... </div>. Но это делается на стороне приложения, а не noscript
  • NOTE предполагается, что перендеринг будет только синхронных видов (только так можно избежать работы с DOM-ом). Была ещё идея сделать у ns.update syncMode: true - чтобы все view считались синхронными и все данные запрашивались тоже синхронной пачкой. Пока не делал.

Copy link
Member

Choose a reason for hiding this comment

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

Может начать собирать Noscript через Browserify? Тогда каждый файл будет отдельным CommonJS модулем и, как следствие, (1) не придется оборачивать каждый файл в IIFE, (2) не придется помнить, что в выражении var ns = ns || require('ns'); первая часть для браузера, а вторая для сервера.

Copy link
Member Author

Choose a reason for hiding this comment

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

А напиши пример, пожалуйста.
К примеру, как будет выглядеть такой вот файл в терминах Browserify:

var ns = ns || require('ns');
ns.coolMethod = function() {
    return 'cool';
};

Copy link
Member

Choose a reason for hiding this comment

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

Вообще, в случае вспомогательной функции или класса, можно не прикреплять его к неймспейсу в самом модуле (чтобы без побочных эффектов):

// cool-method.js
var COOL = 'cool';
module.exports = function coolMethod() {
    return COOL;
};

// ns.js
var ns = {};
ns.coolMethod = require('./cool-method');
module.exports = ns;

Но если без сильных изменений, то можно записать так:

var ns = require('./ns');

module.exports = ns.coolMethod = function() {
    return 'cool';
};

Copy link
Member Author

Choose a reason for hiding this comment

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

Хм хм хм )
Ну может быть и стоит сделать как ты предлагаешь.
/сс @doochik @edoroshenko ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Я за.
И уж точно не стоит писать везде no || require('nommon')

Copy link
Contributor

Choose a reason for hiding this comment

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

да, browserify решает эту проблему

Еще вот такая запись var no = no || require('nommon') не даст нам исполнять код в браузере или сделать модуль, потому что получится вот так

(function() {
  // и тут всегда будет вызываться require, потому что no определен локально
 var no = no || require('nommon');
})()

Copy link
Member Author

Choose a reason for hiding this comment

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

Ну не надо это делать внутри замыкания просто
https://github.com/yandex-ui/noscript/blob/server.render.92/src/ns.view.js#L1-L2

Copy link
Contributor

Choose a reason for hiding this comment

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

Мы сами оборачиваем эти файлы в модули и такие строки все сломают

@edoroshenko
Copy link
Contributor

Давайте относиться к этому pr, как к прототипу. Я здесь пока вижу "решение по-быстрому". Хочется сделать ровно. Я тоже готов сделать заход

@chestozo
Copy link
Member Author

chestozo commented May 5, 2014

Это полноценное решение вообще-то.
Разверни свой комментарий, пожалуйста.

@edoroshenko
Copy link
Contributor

Ром, ну давай по-честному, ты просто запустил ns в ноде и навтыкал костылей, чтобы оно завелось. Я считаю такой подход не очень правильным

@chestozo
Copy link
Member Author

chestozo commented May 5, 2014

Приведи пример костыля?
Я с твоим выводом не согласен, тут нет ни одного костыля.

@edoroshenko
Copy link
Contributor

Примеры костылей: options.syncOnly, options.renderOnly, ns.IS_TOUCH = !no.de && Boolean(..., var ns = ns || require('./ns.js');
Я не согласен с новым интерфейсом update'а.
Я хочу предложить альтернативу, но мне нужно на это какое-то время.

@chestozo
Copy link
Member Author

chestozo commented May 5, 2014

Ну ок, если ты придумаешь более элегантный способ - я не против.
Жду твоего предложения.

@edoroshenko
Copy link
Contributor

Я начал свой заход на серверный ns и теперь могу внести в свою позицию немного конструктива.

@chestozo , ты решил задачу "сделать так, чтобы ns как-то заработал на сервере". Ты её успешно решил. Это proof of concept, прототип, но не рабочее решение. Создание рабочего решения требует более глубокой проработки и проектирования.

Как я вижу правильный подход к решению этой задачи.

  1. Очевидно, нужна отдельная серверная сборка ns. Там точно не нужны ns.action, ns.history. 2. Вместо ns.page на сервере должен быть другой объект (ns.app, ns.server). В этом объекте будут методы, которые вызывают update правильным способом. Нужно прийти к тому, чтобы от разработчика не требовалось ничего переопределять.
  2. В идеале, ns.page на сервере нужно выпилить, но от него зависят view и update. Если от этих зависимостей удастся избавиться, можно будет выпилить. На сервере нет страницы.
  3. ns.init нужно перенести в ns.page, а в серверном объекте сделать свой init.
  4. Нужен отдельный сет тестов, который будет гарантировать, что ns работает нормально в серверном окружении.
  5. Нужен серверный ns.request, в который можно передать функцию, которая будет запрашивать данные.
  6. Нужно сначала решить задачу полностью, а только потом вмердживать эти изменения в master.

Естественно, это только первый подход к снаряду. Следующие шаги, которые я планирую: вникнуть в задачу поглубже, написать тесты, придумать и обсудить api, декомпозировать задачу.

@chestozo
Copy link
Member Author

chestozo commented May 6, 2014

Ты предлагаешь сделать отдельный скрипт для сервера.
Объясни, зачем делать такое сильно разграничение между сервером и клиентом?
Чем для тебя окружение nodejs так сильно отличается от браузера?

/cc @doochik @Lapple тут опять каша завариается, можете добавить что-то к дискуссии?

@edoroshenko
Copy link
Contributor

Объясни, зачем делать такое сильно разграничение между сервером и клиентом?

чтобы на сервер не клиентский код. Либо ты его не тащишь на сервер, либо ты втыкаешь костыли, чтобы он не ломался (if !window итд).

Чем для тебя окружение nodejs так сильно отличается от браузера?

Это же разные окружения. В ноде нет window, location, history, dom, document. Это же очевидно. Это точно нужно перечислять?

Я прелагаю выделить 3 типа сущностей noscript:

  1. Только клиентские (ns.action, ns.history, ns.page, что-то ещё). На сервере они не нужны, поэтому их подключать там вообще не нужно. Они могут сколько угодно завязываться на браузерное окружение (window, history, location).
  2. Только серверные. Таких пока нет, но я думаю, что это должен быть серверный аналог ns.page, но не ns.page (потомучто на сервере нет страницы).
  3. Универсальные (всё остальное). Эти сущности должны быть готовы к тому, что их будут использовать в разных окружениях. Их можно реализовать двумя способами:
  4. Разделить их методы на "универсальные" и "клиентские". Разрешить в клиентских методах завязываться на объекты окружения
  5. Разделить объекты. Например, сделать базовый вид, который работает только в одну сторону - на рендеринг и клиентский, который можно инвалидировать, подписывать итд.

Первый проще, второй - ровнее.

Пока писал этот коммент, придумал третье окружение: тестовое. Нам в бою ни на клиенте, ни на сервере не нужны костыли if window.mocha. Для тестов тоже нужна отдельная сборка.

@edoroshenko
Copy link
Contributor

тут опять каша завариается

Друг, не воспринимай это на свой счёт. Мы все хотим сделать хороший инструмент. Этого можно достичь только дискуссией

@Lapple
Copy link
Member

Lapple commented May 6, 2014

Я думаю, что явно очерченные сборки особо не нужны, они сами органично сформируются.

Предлагаю, с точки зрения Node.js, рассматривать Noscript как набор компонентов (модулей). Да, для браузерного окружения есть клей в виде ns.page и ns.init. Точно такой же клей можно написать и для сервера, но во фреймворк он не попадет, потому что серверный стек у всех наших проектов разный.

Идея с разделением ns.View на два класса мне нравится.

@edoroshenko
Copy link
Contributor

они сами органично сформируются

Это как? )

@Lapple
Copy link
Member

Lapple commented May 6, 2014

Если подключить Browserify, то клиентский ns.init подтянет ns.router, ns.history и ns.action, а ns.page все равно подтянется (он же нужен для старта).

Серверный клей (типо такой) будет подключать ns.router (чтобы URL как-то обработать) и ns.update, чтобы HTML получить.

Copy link
Contributor

Choose a reason for hiding this comment

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

откуда в ns завязка на descript? А если я не хочу descript, а хочу express ?

Copy link
Member Author

Choose a reason for hiding this comment

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

no.de надо читать как node, а не как no.descript.
Это свойство из nommon.
Зависимости от descript - нет )

Copy link

Choose a reason for hiding this comment

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

no.de - так не правильно определять что мы находимся в nodejs.

Если вы будете использовать browserify, то сразу наткнетесь на ошибку.
Лучше использовать такую прверку

Copy link
Contributor

Choose a reason for hiding this comment

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

А ещё лучше - вообще не использовать проверки и писать код, не зависящий от окружения.
Вот не знаю, утопия это в наших обстоятельствах, или нет

Copy link
Member Author

Choose a reason for hiding this comment

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

@maksimr при желании, и эта проверка не будет работать, если в браузере нужный json-чик забацать )
Но и window можно и в nodejs объявить )
В общем - мы все умрём )

Copy link

Choose a reason for hiding this comment

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

@edoroshenko +1

@chestozo Я написал потому, что столкнулся с проблемами при созаднии yate-playground :)

@doochik
Copy link
Contributor

doochik commented May 6, 2014

Мое мнение:

  1. pr имеет право на жизнь, потому что все добавленные в update опции будут потом нужны для prerender и prerequest. Может быть сначала оттолкнуться от них или сразу сделать тут все? Кажется, что эти фичи во многом пересекаются
  2. мне не нравится изменение ns.tmpl, т.к. мы используем этот метод для рендеринга каких-то статичных штук вне дерева. Я бы сделал рядом еще один метод, который генерит строку
  3. Отдельная сборка для node кажется излишеством. Особого оверхеда для ноды нет, зато появляется второй файл, который надо контролировать
  4. Вместо var ns = ns || require() в каждом файле (что нам все сломает) лучше сделать browserify для собранного файла и подключать все сразу, а не по отдельности

@chestozo
Copy link
Member Author

chestozo commented May 6, 2014

мне не нравится изменение ns.tmpl, т.к. мы используем этот метод для рендеринга каких-то статичных штук вне дерева. Я бы сделал рядом еще один метод, который генерит строку

Я вначале так и сделал, а потом переделал назад.
В данный момент в noscript одно единственное место, где вызывается ns.tmpl (+1 в PR).
Мне кажется, иметь дело со строкой html гораздо удобнее, чем с нодой.
А где нужна нода - вызываем ns.html2node.

Вместо var ns = ns || require() в каждом файле (что нам все сломает) лучше сделать browserify для собранного файла и подключать все сразу, а не по отдельности

Вот тут не очень понял, что такое собранный файл.

@doochik
Copy link
Contributor

doochik commented May 6, 2014

  1. если одно место, то почему строка более правильное поведение?
  2. я имею ввиду, что брать dist/noscript.js, оборачивать его и делать module.exports = ns один раз

@edoroshenko
Copy link
Contributor

все добавленные в update опции будут потом нужны для prerender и prerequest

Это нужно решать не через добавление опций к сложному сценарию, а через разделение сценария на отдельные простые, которые потом можно комбинировать в зависимости от окружения.

Особого оверхеда для ноды нет

Не нужно выполнять на сервере специфично клиентский код только из-за того, что нам лень придумывать, как его отделить.

появляется второй файл, который надо контролировать

его тесты будут контролировать

@doochik
Copy link
Contributor

doochik commented May 6, 2014

@edoroshenko не будем, ns.action запускается в ns.init, а его в node никто вызывать не будет

@edoroshenko
Copy link
Contributor

@doochik а зачем вообще создавать объект ns.action?

@edoroshenko
Copy link
Contributor

Я кстати, не две сборки хочу, а три. Ещё для тестов )

@doochik
Copy link
Contributor

doochik commented May 6, 2014

Это не правильно, надо тестировать то, что уходит в браузер, а не какую-то специальную сборку

@chestozo
Copy link
Member Author

chestozo commented May 6, 2014

Вот есть nommon, которые используется в nodejs и в браузере.
Я делал по аналогии. Вроде, работающая схема.
На сервере никому не нужен ns.action, его там и нет.

@edoroshenko
Copy link
Contributor

я хочу в неё перенести только костыли if window.mocha. Да, наверное это не должна быть не сборка, а просто некоторые вещи нужно делать в stub'ах

@edoroshenko
Copy link
Contributor

Нужно из множества работающих схем выбрать оптимальную

@doochik
Copy link
Contributor

doochik commented May 6, 2014

@edoroshenko можно, но тогда все настоящие приваты придется вынести наружу. Но и тогда кастомная сборка не нужна, а просто в стабах все написать можно

@edoroshenko
Copy link
Contributor

@doochik да, всё в стабах

@doochik
Copy link
Contributor

doochik commented May 6, 2014

предлагаю встречу, а то у меня пальцы устали

@edoroshenko
Copy link
Contributor

Парни, давайте я сделаю заход и тогда уже соберёмся и обсудим конкретные решения. А то я пока только критикую и ничего своего не предложил. Я хочу другой api

@doochik
Copy link
Contributor

doochik commented May 6, 2014

Давай все же, как ты любишь, мы сначала поговорим
У меня впечатление, что у нас получается 4 уровня работы update

  1. prerequest: запросить модели
  2. node: 1 + построить дерево + наложить yate и выдать html
  3. prerender: 2 + встроить полученный html в DOM
  4. полный: 3 + вызвать события

@doochik
Copy link
Contributor

doochik commented May 6, 2014

И еще: давайте все споры про встраивание в node, browserify и т.п. оставим на конец. Сначала надо сделать вот эти 4 шага

Потом, уже с готовым решением, проще будет понять че и как делать с нодой

@chestozo
Copy link
Member Author

chestozo commented May 6, 2014

Про разбиение на 4 шага: это неполная картина.

Шаги такие:

  • запрос моделей для синхронных видов
  • запрос моделей для асинхронных видов (для каждого вида - свой)
  • когда пришли синхронные модели - рендер синхронных видов
  • когда приходят модели для асинхронных видов - новый update для каждого асинхронного вида!

Под рендером я понимаю: html + DOM + events.

@chestozo
Copy link
Member Author

chestozo commented May 6, 2014

Я тоже за встречу.

@chestozo
Copy link
Member Author

Если что - вот тут есть пример, как вызывать descript в nodejs https://github.com/pasaran/yate/blob/master/docs/quick-start.md#%D0%92-nodejs

@edoroshenko
Copy link
Contributor

Распилил ns.Update #304

@chestozo
Copy link
Member Author

chestozo commented Jun 8, 2014

Закрываю, раз есть отдельное issue на распил.

@chestozo chestozo closed this Jun 8, 2014
@chestozo chestozo deleted the server.render.92 branch May 5, 2018 11:03
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.

6 participants