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

default export in gatsby-ssr/browser doesn't produce warnings #21382

Closed
pieh opened this issue Feb 11, 2020 · 1 comment · Fixed by #23133
Closed

default export in gatsby-ssr/browser doesn't produce warnings #21382

pieh opened this issue Feb 11, 2020 · 1 comment · Fixed by #23133
Assignees

Comments

@pieh
Copy link
Contributor

pieh commented Feb 11, 2020

Right now we produce error when there are named export in gatsby-ssr or gatsby-browser that don't match any of our API hooks ( https://www.gatsbyjs.org/docs/ssr-apis/ / https://www.gatsbyjs.org/docs/browser-apis/)

For example:

// in gatsby-ssr.js
export const notExistingAPIHook = () => {
  console.log("this won't show")
}

will produce

 ERROR #11329 

Your plugins must export known APIs from their gatsby-ssr.js.

See https://www.gatsbyjs.org/docs/ssr-apis/ for the list of Gatsby ssr APIs.

- Your local gatsby-ssr.js is using the API "notExistingAPIHook" which is not a known API.

When there is default export:

// in gatsby-ssr.js
export default () => {
  console.log("this won't show")
}

We don't get any warnings or errors, but it won't ever be executed by gatsby. Behaviour for exports should be consistent and we should warn about default exports as well

Task

Adjust

// extract names of exports from file
traverse(ast, {
// Check if the file is using ES6 imports
ImportDeclaration: function ImportDeclaration(astPath) {
isES6 = true
},
// get foo from `export const foo = bar`
ExportNamedDeclaration: function ExportNamedDeclaration(astPath) {
const exportName = get(
astPath,
`node.declaration.declarations[0].id.name`
)
isES6 = true
if (exportName) exportNames.push(exportName)
},
// get foo from `export { foo } from 'bar'`
// get foo from `export { foo }`
ExportSpecifier: function ExportSpecifier(astPath) {
const exportName = get(astPath, `node.exported.name`)
isES6 = true
if (exportName) exportNames.push(exportName)
},
AssignmentExpression: function AssignmentExpression(astPath) {
const nodeLeft = astPath.node.left
if (nodeLeft.type !== `MemberExpression`) return
// ignore marker property `__esModule`
if (get(nodeLeft, `property.name`) === `__esModule`) return
// get foo from `exports.foo = bar`
if (get(nodeLeft, `object.name`) === `exports`) {
isCommonJS = true
exportNames.push(nodeLeft.property.name)
}
// get foo from `module.exports.foo = bar`
if (
get(nodeLeft, `object.object.name`) === `module` &&
get(nodeLeft, `object.property.name`) === `exports`
) {
isCommonJS = true
exportNames.push(nodeLeft.property.name)
}
},
})

to handle default exports as well and ideally add test case for that in https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/src/bootstrap/__tests__/resolve-module-exports.js

@danilobuerger
Copy link
Contributor

Related to this, when keeping the ssr code somewhere else, including it like:

export * from "./src/utils/gatsby-ssr";

fails silently too.

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 a pull request may close this issue.

4 participants