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

(fix): templates should not have baseUrl or paths set #707

Merged

Conversation

agilgur5
Copy link
Collaborator

@agilgur5 agilgur5 commented May 3, 2020

  • there are many issues about TS path aliases not fully "working", so
    they should be removed from the templates until they are completely
    "working"

    • instead of shipping templates with a partially "broken" and
      confusing configuration enabled
    • especially confusing to users because editors will auto-complete
      with the absolute path instead of the relative path
    • and the fact that these paths are unresolved at build or test time
      means that it's not understood that this is "broken" until later in
      the process
  • this is a very common misconception with TS users that paths
    rewrites/transforms imports during compilation, but it actually does
    not change any absolute paths to relative, it leaves them as is

    • it just resolves type information to help on platforms that import
      differently
      • basically, it's somewhat of a legacy option, but folks think it's
        used for aliases and does transforms
        • and folks seem to want it to work that way too
        • it actually does the reverse, it supports the usage of
          aliases/import rewrites elsewhere
    • unfortunately this misconception made its way into the very first
      template, causing lots of confusion and issues reported since
  • so paths isn't "broken" per se, it just doesn't do what people
    think it does (including template authors)

    • well it is broken for standard TSDX usage out-of-the-box
      • since TSDX doesn't output for platforms where it makes sense
        out-of-the-box
  • as a result, for aliases to be fully supported by TSDX, we'd have to
    add some sort of transformation(s) for build, test, and lint

    • until then this shouldn't be in the templates
      • but don't give an error on it because there are
        methods/"workarounds" to support aliasing/import rewrites using
        the configuration in paths
        • and if one is using import rewrites, paths is necessary for
          TS to not give errors
  • also removes an inconsistency in the Storybook template that used
    "@/*" as an alias, even though the other templates didn't have that

    • while this is a supported alias by some libraries in the community,

    • oversight in code review when Storybook templates were added and
      caused some confusion for users

  • also removes these options from all test fixtures


Long time coming for this one, these have been in the codebase since the very first template was added in 9010ecb and have caused much confusion since then.

Fixes the root cause of #91, #699, #516, #139, #133, etc, but doesn't add an import/rewrite mechanism, which has been requested in some of those issues. Does fix confusion / bugs due to the faulty templates. Specifically:

Full support for aliases, i.e. an import rewrite/transform mechanism, will be added eventually, keep an eye on #91 for that.

- there are many issues about TS path aliases not fully "working", so
  they should be removed from the templates until they are completely
  "working"
  - instead of shipping templates with a partially "broken" and
    confusing configuration enabled
  - especially confusing to users because editors will auto-complete
    with the absolute path instead of the relative path
  - and the fact that these paths are unresolved at build or test time
    means that it's not understood that this is "broken" until later in
    the process

- this is a *very* common misconception with TS users that `paths`
  rewrites/transforms imports during compilation, but it actually does
  not change any absolute paths to relative, it leaves them as is
  - it just resolves type information to help on platforms that import
    differently
    - basically, it's somewhat of a legacy option, but folks think it's
      used for aliases and does transforms
      - and folks seem to want it to work that way too
      - it actually does the reverse, it supports the usage of
        aliases/import rewrites elsewhere
  - unfortunately this misconception made its way into the very first
    template, causing lots of confusion and issues reported since

- so `paths` isn't "broken" per se, it just doesn't do what people
  think it does (including template authors)
  - well it *is* broken for standard TSDX usage out-of-the-box
    - since TSDX doesn't output for platforms where it makes sense
      out-of-the-box

- as a result, for aliases to be fully supported by TSDX, we'd have to
  add some sort of transformation(s) for build, test, and lint
  - until then this shouldn't be in the templates
    - but don't give an error on it because there are
      methods/"workarounds" to support aliasing/import rewrites using
      the configuration in `paths`
      - and if one is using import rewrites, `paths` is necessary for
        TS to not give errors

- also removes an inconsistency in the Storybook template that used
  "@/*" as an alias, even though the other templates didn't have that
  - while this is a supported alias by some libraries in the community,

  - oversight in code review when Storybook templates were added and
    caused some confusion for users

- also removes these options from all test fixtures
@agilgur5 agilgur5 added the topic: TS Paths Aliases Related to using aliases with TypeScript paths label May 3, 2020
@agilgur5 agilgur5 added this to the v0.13.x milestone May 3, 2020
Copy link
Collaborator Author

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

LGTM. Mostly just the same 4 lines removed everywhere

@agilgur5 agilgur5 merged commit 17ffcd2 into jaredpalmer:master May 3, 2020
velut added a commit to velut/short-time-ago that referenced this pull request Aug 29, 2020
velut added a commit to velut/query-registry that referenced this pull request Aug 29, 2020
@agilgur5 agilgur5 added the kind: bug Something isn't working label Oct 1, 2020
paul-vd pushed a commit to EezyQuote/tsdx that referenced this pull request Dec 1, 2020
- there are many issues about TS path aliases not fully "working", so
  they should be removed from the templates until they are completely
  "working"
  - instead of shipping templates with a partially "broken" and
    confusing configuration enabled
  - especially confusing to users because editors will auto-complete
    with the absolute path instead of the relative path
  - and the fact that these paths are unresolved at build or test time
    means that it's not understood that this is "broken" until later in
    the process

- this is a *very* common misconception with TS users that `paths`
  rewrites/transforms imports during compilation, but it actually does
  not change any absolute paths to relative, it leaves them as is
  - it just resolves type information to help on platforms that import
    differently
    - basically, it's somewhat of a legacy option, but folks think it's
      used for aliases and does transforms
      - and folks seem to want it to work that way too
      - it actually does the reverse, it supports the usage of
        aliases/import rewrites elsewhere
  - unfortunately this misconception made its way into the very first
    template, causing lots of confusion and issues reported since

- so `paths` isn't "broken" per se, it just doesn't do what people
  think it does (including template authors)
  - well it *is* broken for standard TSDX usage out-of-the-box
    - since TSDX doesn't output for platforms where it makes sense
      out-of-the-box

- as a result, for aliases to be fully supported by TSDX, we'd have to
  add some sort of transformation(s) for build, test, and lint
  - until then this shouldn't be in the templates
    - but don't give an error on it because there are
      methods/"workarounds" to support aliasing/import rewrites using
      the configuration in `paths`
      - and if one is using import rewrites, `paths` is necessary for
        TS to not give errors

- also removes an inconsistency in the Storybook template that used
  "@/*" as an alias, even though the other templates didn't have that
  - while this is a supported alias by some libraries in the community,

  - oversight in code review when Storybook templates were added and
    caused some confusion for users

- also removes these options from all test fixtures
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working topic: TS Paths Aliases Related to using aliases with TypeScript paths
Projects
None yet
1 participant