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

Feature: Destructured imports in variable declarations, Refactor parser with Babel type guards #103

Conversation

MajorLift
Copy link

@MajorLift MajorLift commented Dec 16, 2021

Love this project, and having a ton of fun going through the code!

1) Features

  • Newly supports a subset of require() syntax

    • DOES support object destructuring with alias assignment
      • e.g. const { foo: alias, bar } = require("./module")
    • DOES support array destructuring
      • e.g. const [foo, bar] = require("./module")
    • DOES NOT support namespaces and member expressions
      • e.g. const foo = require("./module"); ... ; <foo.component />;
    • DOES NOT support invocations or method/member calls
      • e.g. const foo = require("./module")();
      • e.g. const foo = require("./module").method(arg);
  • Newly supports a subset of variable declaration imports

    • DOES support object destructuring with alias assignment, and array destructuring
      (no actual use cases, but intermediate step towards fully supporting dynamic imports)
      • e.g. const { foo: alias, bar } = import("./module")
      • e.g. const [foo, bar] = import("./module")
    • DOES NOT support await expressions, .then() chains, or Promise.resolve() calls for dynamic imports
      • e.g. const foo = await import("./module")
      • e.g. import("./module").then(mod => mod.loadPage())
      • e.g. const foo = Promise.resolve(import("./module"))

2) SaplingParser

  • Extensive rewrites in .getImports(), .findVarDecImports() methods.
  • Explicit checks with strong typing for all control flow branches in import statements.
  • Passes all previously written tests.
  • Readability, modularity, extensibility improvements.

3) Strict Typescript linter settings

  • Strong typing and stricter checks.
  • Promotes best practices.

Water for the tree :)

- Enabled strict mode for extension.
- Relaxed some superfluous style rules.
- Set eslint scope to include webviews/.
- Record<string, unknown> is default recommended by @typescript-eslint/ban-types.
- Using Array<T> when T is composite data type, and T[] when T is primitive is useful visual cue.
- test 14: Subset of require() syntax
a) object destructuring with alias assignment
b) array destructuring

- test 15: Subset of variable declaration import
a) object destructuring alias assignment
b) array destructuring
- Extensive rewrites in getImports(), findVarDecImports() methods
- Explicit checks with strong typing for most import statement variations
- Readability, modularity, extensibility improvements
- Newly supports a subset of require() syntax
a) DOES support object destructuring with alias assignment
  * e.g. const { foo: alias, bar } = require("./module")
b) DOES support array destructuring
  * e.g. const [foo, bar] = require("./module")
c) DOES NOT support namespaces and member expressions
  * e.g. const foo = require("./module"); ... ; <foo.component />;
d) DOES NOT support invocations or method/member calls
  * e.g. const foo = require("./module")();
  * e.g. const foo = require("./module").method(arg);

- Newly supports a subset of variable declaration imports
a) DOES support object destructuring with alias assignment, and array destructuring
  * no actual use cases, but intermediate step towards fully supporting dynamic imports
  * e.g. const { foo: alias, bar } = import("./module")
  * e.g. const [foo, bar] = import("./module")
- DOES NOT support await expressions, .then() chains, or Promise.resolve() calls for dynamic imports
  * e.g. const foo = await import("./module")
  * e.g. import("./module").then(mod => mod.loadPage())
  * e.g. const foo = Promise.resolve(import("./module"))
- reserve parentheses for capture+backreference where possible
- convention for descriptive variable naming
- encountered issues with testing resolved by purging out/, dist/
@MajorLift MajorLift force-pushed the feature/parser/type-guards,require,variable-declaration-imports branch from 0937c64 to ad75e04 Compare December 21, 2021 17:54
@jdgreenberger
Copy link

@PLCoster is anyone maintaining this repo? It seems like a really useful tool, but is not currently very useful without addressing this issue, which may be solved by this PR.

@MajorLift
Copy link
Author

MajorLift commented May 24, 2022

This PR lays some groundwork for eventually supporting barrel files, but isn't a full fix for the issue.

@MajorLift MajorLift closed this Jun 8, 2022
@MajorLift MajorLift deleted the feature/parser/type-guards,require,variable-declaration-imports branch February 28, 2023 06:11
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