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

chore(gatsby): upgrade latest-version #36434

Merged
merged 8 commits into from Oct 18, 2022
Merged

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Aug 22, 2022

Description

This updates to latest of ... latest-version - this package requires node@>=14.16, which we couldn't do in v4 due to us being on minimal node@14.15

Related Issues

[ch54713]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Aug 22, 2022
@pieh pieh added type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Aug 22, 2022
@pieh pieh marked this pull request as draft August 22, 2022 09:57
@pieh
Copy link
Contributor Author

pieh commented Aug 22, 2022

This package is ESM-only now and requires additional changes to how it's imported and used

@pieh pieh added the status: blocked This issue/PR can't be solved at the moment and shouldn't be closed/merged label Aug 22, 2022
@LekoArts LekoArts added this to the Gatsby 5 milestone Oct 12, 2022
@pieh pieh force-pushed the chore/upgrade/latest-version branch from 28b0fcf to 66dd212 Compare October 17, 2022 11:36
@pieh pieh marked this pull request as ready for review October 17, 2022 12:05
@pieh pieh removed the status: blocked This issue/PR can't be solved at the moment and shouldn't be closed/merged label Oct 17, 2022
Comment on lines -6 to +8
presets: [["babel-preset-gatsby-package"]],
presets: [["babel-preset-gatsby-package", {
keepDynamicImports: [`./src/utils/feedback.ts`]
}]],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this PR adds a way to mark individual files to not transform

await import("")

to

await Promise.resolve().then(() => _interopRequireWildcard(require("")));

and instead keep dynamic import

We have plenty of dynamic imports in our codebase / gatsby package and switching it everywhere (through already existing esm: true option that babel-preset-gatsby-package) doesn't seem like good idea and definitely would require solid testing.

This approach allows us to address individual modules and we can over-time migrate more things if needed

[path.resolve(__dirname, "../../babel-transform-compiler-flags.js"),
[path.resolve(__dirname, "../../../babel-transform-compiler-flags.js"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was moved from top-level for all fixtures to one directory below that, hence path adjustment

function _interopRequireWildcard(obj, nodeInterop) { if (!nodeInterop && obj && obj.__esModule) { return obj; } if (obj === null || typeof obj !== "object" && typeof obj !== "function") { return { default: obj }; } var cache = _getRequireWildcardCache(nodeInterop); if (cache && cache.has(obj)) { return cache.get(obj); } var newObj = {}; var hasPropertyDescriptor = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var key in obj) { if (key !== "default" && Object.prototype.hasOwnProperty.call(obj, key)) { var desc = hasPropertyDescriptor ? Object.getOwnPropertyDescriptor(obj, key) : null; if (desc && (desc.get || desc.set)) { Object.defineProperty(newObj, key, desc); } else { newObj[key] = obj[key]; } } } newObj.default = obj; if (cache) { cache.set(obj, newObj); } return newObj; }

async function someFunction() {
return await Promise.resolve().then(() => _interopRequireWildcard(require(`path`)));
Copy link
Contributor Author

@pieh pieh Oct 17, 2022

Choose a reason for hiding this comment

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

ensuring default doesn't change to not introduce accidental regressions

exports.someFunction2 = someFunction2;

async function someFunction2() {
return await import(`path`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this file was marked to keep dynamic import in

[path.resolve(__dirname, "../../../index.js"),
{
keepDynamicImports: [`./packages/babel-preset-gatsby-package/lib/__tests__/fixtures/keep-dynamic-import/with-override/input.js`]
}
],
],
and it in fact keep that instead of doing transformation

@LekoArts LekoArts merged commit eb9a330 into master Oct 18, 2022
@LekoArts LekoArts deleted the chore/upgrade/latest-version branch October 18, 2022 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants