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

fix(gatsby): [rendering engines] use results of exports removal if sourceMap was not generated alongside transformed code #37282

Merged

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Dec 16, 2022

Description

We use custom loader to remove gatsby-node lifecycles that are not relevant to engines runtime in

{
// specific set of loaders for gatsby-node files - our babel transform that removes lifecycles not needed for engine -> relocator
test: /gatsby-node\.(cjs|mjs|js|ts)$/,
// it is recommended for Node builds to turn off AMD support
parser: { amd: false },
use: [
assetRelocatorUseEntry,
{
loader: require.resolve(
`../../utils/webpack/loaders/webpack-remove-exports-loader`
),
options: {
remove: getApisToRemoveForQueryEngine(),
jsx: false,
},
},
],
},

This however is currently not always working as expected as the loader currently expect sourceMap to be generated alongside transformed code before using the transformed code. If map is not generated we discard transformed code.

This PR allow usage of transformed code if it's not accompanied by sourcemap - we will only do passthrough if transformed code is not generated.

Documentation

Related Issues

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 16, 2022
@@ -18,9 +18,11 @@ node node_modules/gatsby/dist/schema/graphql-engine/standalone-regenerate.js
*/

import { createGraphqlEngineBundle } from "./bundle-webpack"
import { createPageSSRBundle } from "./../../utils/page-ssr-module/bundle-webpack"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changes in packages/gatsby/src/schema/graphql-engine/standalone-regenerate.ts are unrelated to fix itself, just some dev tools update that I did as part of my investigation that I think make sense to commit in general - I can drop this change from this PR (and either create separate or just skip pushing it)

@pieh pieh added topic: render-mode Related to Gatsby's different rendering modes and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 16, 2022
Comment on lines -52 to 56
} else if (result && result.code && result.map) {
callback(null, result?.code, result?.map)
} else if (result && result.code) {
callback(null, result?.code, result?.map ?? undefined)
} else {
callback(null, source, sourceMap)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actual fix here - sometimes we will get result.code with falsyt result.map, this change make sure we use that and only fallback to pass original source (and sourceMap) if transformed code is falsy or we get no result at all (not sure if that can ever happen without error)

@pieh pieh marked this pull request as ready for review December 16, 2022 13:41
@pieh pieh added this to To cherry-pick in V4 Release hotfixes via automation Dec 16, 2022
@pieh pieh added this to To cherry-pick in V5 Release hotfixes via automation Dec 16, 2022
@pieh pieh merged commit 770748d into master Dec 16, 2022
@pieh pieh deleted the engines/use-transformed-code-even-if-sourcemap-is-not-available branch December 16, 2022 16:05
@pieh pieh moved this from To cherry-pick to Backport PR opened in V5 Release hotfixes Dec 20, 2022
pieh added a commit that referenced this pull request Dec 20, 2022
…urceMap was not generated alongside transformed code (#37282)

* fix(gatsby): [rendering engines] use results of exports removal if sourceMap was not generated alongside transformed code

* chore: regenerate also page-ssr bundle in standalone-regenerate

(cherry picked from commit 770748d)
pieh added a commit that referenced this pull request Dec 20, 2022
…urceMap was not generated alongside transformed code (#37282)

* fix(gatsby): [rendering engines] use results of exports removal if sourceMap was not generated alongside transformed code

* chore: regenerate also page-ssr bundle in standalone-regenerate

(cherry picked from commit 770748d)
@pieh pieh moved this from To cherry-pick to Backport PR opened in V4 Release hotfixes Dec 20, 2022
pieh added a commit that referenced this pull request Dec 20, 2022
…urceMap was not generated alongside transformed code (#37282) (#37296)

* fix(gatsby): [rendering engines] use results of exports removal if sourceMap was not generated alongside transformed code

* chore: regenerate also page-ssr bundle in standalone-regenerate

(cherry picked from commit 770748d)

Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
pieh added a commit that referenced this pull request Dec 20, 2022
…urceMap was not generated alongside transformed code (#37282) (#37299)

* fix(gatsby): [rendering engines] use results of exports removal if sourceMap was not generated alongside transformed code

* chore: regenerate also page-ssr bundle in standalone-regenerate

(cherry picked from commit 770748d)

Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
@pieh pieh moved this from Backport PR opened to Published in V5 Release hotfixes Dec 20, 2022
@pieh pieh moved this from Backport PR opened to Published in V4 Release hotfixes Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: render-mode Related to Gatsby's different rendering modes
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants