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

feat: Add import meta #141

Merged
merged 7 commits into from
May 20, 2024
Merged

feat: Add import meta #141

merged 7 commits into from
May 20, 2024

Conversation

Brayan-724
Copy link
Contributor

This Fixes #140.

In this pr is implemented the expression import.meta in parser and checker. Also fix some parsing errors related to dynamic import.

Primary features

import.meta is setted to type ImportMeta.
import.meta.env is setted to type ImportEnv.

Secondary fix

Before, dynamic import was searching OpenParentheses instead of CloseParentheses.

const _ = import("file");
                       ^ Expected OpenParentheses, ...

Before, dynamic import search for string literals, but do nothing in other cases.
After, dynamic import throws error if encounter another thing

This opens an issue. Limit dynamic import to string literals, no variables or expressions

Copy link
Owner

@kaleidawave kaleidawave left a comment

Choose a reason for hiding this comment

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

Awesome this is an important feature! Thanks for opening an issue and trying to tackle it. Parsing logic looks great. Have left a couple of changes needed to make it perfect!

I was going to make a comment about looking into type checking it but it looks like you have already started! What you have nearly works just needs a few changes I have requested for it to work.

Once you have done that, could you add a simple test for import.meta and dynamic import with and without a string expression and with and without an options object to https://github.com/kaleidawave/ezno/blob/main/parser/tests/expressions.rs

And for type checking you can add something like import.meta.x satisfies number and show it throws a type error like Expected number, found string | undefined to here: https://github.com/kaleidawave/ezno/blob/main/checker/specification/specification.md#imports-and-exports

If you are busy then LMK and I can do it. Otherwise once those changes have been made and tests added for them it should be good to merge!

parser/src/expressions/mod.rs Outdated Show resolved Hide resolved
parser/src/expressions/mod.rs Outdated Show resolved Hide resolved
} else {
None
};
let end = reader.expect_next(TSXToken::CloseParentheses)?;
Copy link
Owner

Choose a reason for hiding this comment

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

👍 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Owner

Choose a reason for hiding this comment

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

👍
image

checker/src/types/mod.rs Outdated Show resolved Hide resolved
checker/src/synthesis/expressions.rs Outdated Show resolved Hide resolved
checker/src/types/store.rs Outdated Show resolved Hide resolved
checker/definitions/simple.d.ts Outdated Show resolved Hide resolved
@kaleidawave kaleidawave added parser Related to Ezno's syntax parser, AST definitions and output bug Something isn't working enhancement New feature or request checking Issues around checking labels May 17, 2024
@Brayan-724
Copy link
Contributor Author

Thanks for review my contribution, your feedback and suggestions are so clear. I'm working on.

@kaleidawave
Copy link
Owner

All is working, just

  • Fixed the interface confusion
  • Added the type checking test
  • Got a syntax error because the parser saw a statement beginning with import and went to the statement version rather than looking at the token after and realising it was a import.meta. Fixed that in the parser logic 🎉

Ignoring the fuzzing issue which is from separate I was working on. Everything is good to go. Thanks for the change!

@kaleidawave kaleidawave merged commit 47a423a into kaleidawave:main May 20, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working checking Issues around checking enhancement New feature or request parser Related to Ezno's syntax parser, AST definitions and output
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parsing import meta
2 participants