-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Transpile code using esbuild and update some dependencies #567
Conversation
As recommended here: node-fetch/node-fetch#1167
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[придирка] Чисто из гигиенических соображений я бы рекомендовал не подмешивать несвязанные изменения типа апгрейдов зависимостей. Проще ревьюить, проще откатывать.
const { pathToFileURL } = require("url"); | ||
|
||
module.exports = function (code, filename) { | ||
// Replace import.meta.url in gifsicle package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему это надо делать?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
О, тут очень всё интересно. Gifsycle-пакет выдаёт путь к gifsycle-бинарнику, эти бинарники у него лежат в папочке. Пакет перешёл на esm, а в esm нельзя просто сказать __dirname, чтобы узнать, где твои файлы. Надо использовать import.meta.url, который выдаёт URL (file://…) текущего файла. import.meta.url — штука принципиально динамическая, и работает только в esm-контексте. Но мы все наши файлы транспилируем в cjs (потому что большинство node-пакетов в cjs и всякие тулзы типа mocha ожидают cjs). Поэтому import.meta.url в коде gifsycle перестаёт работать при транспиляции. Единственный выход — тупо его заменить на нужное значение (примерно как DefinePlugin работает), что тут и делается.
host: defer((cfg) => cfg.host), | ||
options: {}, | ||
adminRecipient: { email: '[email protected]', screenName: 'Pepyatka admin' }, | ||
// Subjects | ||
resetPasswordMailSubject: 'Pepyatka password reset', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мы же не шлём слово Pepyatka в заголовках писем про восстановление пароля?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не шлём. у нас кастомный конфиг, а этот остался в наследство по-умолчанию. нужно переделать конечно
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут можно переделать по тому же принципу, что и новые subject-ы
|
||
describe('BestOfDigests', () => { | ||
describe('renderSummaryBody', () => { | ||
it(`should render a summary body and doesn't blow up`, async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если мы хотим проверить, не взорвалось ли наше приложение, то, возможно, стоит проверять, не вызывался ли в процессе теста console.error или console.warn.
This PR removes 'esbuild-register' in favor of a custom transpilation algorithm based on the 'esbuild-register' and 'esbuild-runner' ones. This algorithm allows to transpile all existing dependencies and, if necessary, to patch the source code of packages.
There are also some major updates of 'unexpected', 'chai', 'koa-body' etc.
Also, a very simple test for Best Of Digest email creation was added, just to be sure that the React renderer works with the new transpiler.