Skip to content

Only register exported functions as server references#39

Merged
unstubbable merged 1 commit into
mainfrom
fix-register-server-references
Feb 16, 2024
Merged

Only register exported functions as server references#39
unstubbable merged 1 commit into
mainfrom
fix-register-server-references

Conversation

@unstubbable

Copy link
Copy Markdown
Owner

No description provided.

@vercel

vercel Bot commented Feb 16, 2024

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mfng ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 16, 2024 9:25am


if (
t.isIdentifier(id) &&
(t.isArrowFunctionExpression(init) || t.isFunctionExpression(init))

@lubieowoce lubieowoce Feb 16, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

will this catch

'use server'
async function foo() { ... };
export const bar = foo;

?
FWIW i think next just does

if (typeof NAME === 'function') {
  registerServerReference(NAME, 'NAME' ...);
} else {
  errorBadExport('NAME');
}

for all exports because it's hard to analyze this statically

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

No, this does not cover all possible scenarios. It's pretty opinionated regarding what code style it supports, e.g. I also don't want to support default exports.

@unstubbable unstubbable Feb 16, 2024

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

...or inline 'use server' directives

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

if (typeof NAME === 'function') {
  registerServerReference(NAME, 'NAME' ...);
} else {
  errorBadExport('NAME');
}

Hmm, I am considering this now...
Thanks for bringing this to my attention.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

But why would you error in the else case?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i think it's because the expectation is that a "use server" module exports server references, and importing other things from it might act weird, because they won't get tranformed. like export const foo = 1 would work, but arbitrary non-function exports might not -- what if someone does export const dbClient = myDb.connect()? so easier to just blanket-ban than let the user run into puzzling errors i think

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Yeah, makes sense.

@unstubbable
unstubbable merged commit 3c9bc36 into main Feb 16, 2024
@unstubbable
unstubbable deleted the fix-register-server-references branch February 16, 2024 10:01
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 this pull request may close these issues.

2 participants