-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
chore(gatsby): migrate webpack-hmr-hooks-patch to TypeScript #22280
chore(gatsby): migrate webpack-hmr-hooks-patch to TypeScript #22280
Conversation
@blainekasten I investigated the reason why CI failed. It's because the transpiled
"use strict";
const originalFetch = global.fetch;
delete global.fetch;
module.exports = require(`react-hot-loader/webpack`);
global.fetch = originalFetch;
//# sourceMappingURL=webpack-hmr-hooks-patch.js.map On the other hand, "use strict";
var _interopRequireDefault = require("@babel/runtime/helpers/interopRequireDefault");
exports.__esModule = true;
exports.default = void 0;
var _webpack = _interopRequireDefault(require("react-hot-loader/webpack"));
exports.default = _webpack.default;
/* eslint-disable @typescript-eslint/ban-ts-ignore */
// @ts-ignore: This file assume run at browsers.
const originalFetch = global.fetch; // @ts-ignore
delete global.fetch;
global.fetch = originalFetch;
//# sourceMappingURL=webpack-hmr-hooks-patch.js.map So, the order of deleting and restoring |
@sasurau4 thank you so much for your exploration here. I think you are right that this file ultimately should remain as a JS file. So here is what I think we should do. Let's add a comment to the top of the file that says this file should remain as a JS file, with a reason why and link to this PR. I'd be happy to merge that so we know in the future why this one file stayed a JS file. |
@blainekasten OK, Thanks 👍 |
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.
This looks great to me, let's merge it!
Thank you so much for contributing to our TypeScript refactor! We have more work to do and we would love to have you stay involved in our transition. Please submit more PRs! 💜
Description
Part of #21995
I think the patch assume to run on browsers and use
eslint-disable
and@ts-ignore
. If you have any other ideas, I hope you would pointed it out.Documentation
Related Issues
Part of #21995