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

Design Meeting Notes, 8/4/2021 #45328

Closed
DanielRosenwasser opened this issue Aug 4, 2021 · 1 comment
Closed

Design Meeting Notes, 8/4/2021 #45328

DanielRosenwasser opened this issue Aug 4, 2021 · 1 comment
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

CommonJS Emit Breaks in 4.4

#45189

  • Affects both CommonJS and AMD.

  • Today, you can write

    import { logSomething } from "./class";
  • Have to ensure that when invoking logSomething, its this value is correct.

  • However, some edge cases.

    class C {
        logSomething() {
            this.log("something");
        }
    
        log(message: string) {
            console.log(message)
        }
    }
    const c = new C();
    export = c;
  • Will fail at runtime, since this.log will run as undefined.log.

    • We don't flag this as an error.
  • We do tell people to use a default import in some cases.

    • export = new C()
    • const c = new C();
      export = c;
  • Best thing when targeting commonjs is import thing = require("./class") and reference thing.logSomething.

  • Could go through DefinitelyTyped, explicitly force a this: parameter.

    • Have to manually annotate.
  • Really is about implicit this parameters being missing.

  • We just don't have a lot of feedback and we feel nervous about that.

  • less seems to be a big library that does this, should help protect users in the .d.ts files.

  • Conclusion

    • Keep in for 4.4
    • Document in breaking changes
    • Have someone make a PR to less on DefintelyTyped.

Splitting out DOM Event Types

#43972

  • Maintainers of the Node.js types have mentioned that it's very difficult to avoid conflicts between the DOM and Node, especially since they often have many common declarations.
  • Recently got hit with conflicts in AbortSignal.abort.
  • Consider finding the shared set of types in DOM which are always used in @types/node.
    • Move that into its own common types.
    • Let @types/node depend on that, let the DOM types depend on that.
  • So this would be a new lib entry?
    • Yes.
  • Feel like to use @types/node, you'd need to say dom.events in your lib.
    • But Node would avoid this by writing /// <reference lib="dom.events />
  • Probably seems like a good thing we want to do
    • Benefits users
    • Benefits DefinitelyTyped's Node.js maintainers
    • Is complexity that we can't fully reason about.
  • Does any human have all of this in their head?
    • It's discussed a bit in the issue.
    • caniusethis
  • Have to experiment to make sure that these types are "the same enough".
    • As soon as something slightly diverges, we're screwed.
  • Also, can't iterate as quickly because the Node types are locked to the next versions of TS.
  • Thinking about WebWorkers - does that play into this? Do Workers have the same primitives?
    • Can't speak to this as much, but it will definitely impact WebWorkers.
  • Do we need an investigation?
    • Could give it a shot and see what it looks like.
  • Node.js is definitely trying to bridge together environments, so finding ways to declare shared things is tough.
  • Could get by with more clever merging.
  • Conclusion
    • Want to see what this feels like, see if we can spot the risks there.
@andrewbranch
Copy link
Member

andrewbranch commented Sep 23, 2021

CommonJS Emit Breaks in 4.4

Got another example filed: #46027

#45189 was marked “working as intended” because the emit was correct, but I feel like it’s not quite fair to imply that we feel fine about how many places we let you write a totally bogus named import + call, so I’m leaving #46027 open to focus on the lack of an error, though it does seem pretty much intractable without untenable levels of breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Notes Notes from our design meetings
Projects
None yet
Development

No branches or pull requests

2 participants