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

[draft] feat: Allow props to be exported from other files #122

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

alii
Copy link

@alii alii commented Apr 1, 2022

Fixes #121 and allows for props to exported in the following manner

export {getStaticProps} from '../props';

export default function Page(props) {
	// ...
}

@alii alii changed the title feat: Allow props to be exported from other files [draft] feat: Allow props to be exported from other files Apr 1, 2022
@alii
Copy link
Author

alii commented Apr 4, 2022

Okay so, the test is passing at this point so just wanted to ask for any feedback, maybe we could implement some more test cases?

cc @Skn0tt @flybayer

@Skn0tt
Copy link
Member

Skn0tt commented Apr 6, 2022

@alii thanks a lot for working on this! There's a lot of changes unrelated to the original issue, that's making it hard for me to review. Can I ask you to revert the import changes you made, so this PR is more contained? That'd give me the confidence that I'm not missing anything while reviewing.

@alii
Copy link
Author

alii commented Apr 6, 2022

@Skn0tt The import changes were due to a type mismatch between @babel/core and @babel/types — they're the same functionally just without having conflicting types (for whatever reason that was). I can revert the imports but it will leave a few anys and things lying around.

@Skn0tt
Copy link
Member

Skn0tt commented Apr 8, 2022

Interesting! Yeah, reverting would be nice.

@Skn0tt
Copy link
Member

Skn0tt commented Apr 8, 2022

I'll look at that type mismatch separately, that sounds interesting

@apieceofbart
Copy link

@Skn0tt do you need some help with this one? I would love to see it merged!

@Skn0tt
Copy link
Member

Skn0tt commented Sep 19, 2022

@apieceofbart this PR contains a lot of unrelated changes, which is why I'm not moving forward with it. if you want to work on it, I think it'd be best to open a new PR :)

@apieceofbart
Copy link

@apieceofbart this PR contains a lot of unrelated changes, which is why I'm not moving forward with it. if you want to work on it, I think it'd be best to open a new PR :)

cool, yeah, I'll give it a try especially that a lot of code is already written :)

@alii
Copy link
Author

alii commented Sep 19, 2022

Ohh I'm really sorry haha, I'm not sure why I never completed this. I think I did have the changes locally but didn't push for some reason. Happy to help as well @apieceofbart if you need a hand. :D

@apieceofbart
Copy link

apieceofbart commented Sep 20, 2022

@alii I only checked briefly yesterday - tried to be smart and copy your code but without the type changes part. Unfortunately when I did this there were a couple of TS errors.

What is weird is that when I open index.ts without any changes I get error in vscode here and here: Argument of type 'Identifier' is not assignable to parameter of type 'Expression | V8IntrinsicIdentifier'.
However running tsc from code line is fine.
This is not a big deal but a bit annoying since when working on the code you don't know which errors are "real".

@alii
Copy link
Author

alii commented Sep 20, 2022

I think that was the reason I ended up updating all of the dependencies in my branch, because there were a lot of type mismatches between them. @apieceofbart

iirc, the things I deps I moved around here fixed those issues

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.

Shared SSR/SSG hooks are not transformed
3 participants