Giter VIP home page Giter VIP logo

Comments (8)

sinclairzx81 avatar sinclairzx81 commented on July 2, 2024

@akim-bow Hi,

Turns out that ESM and CJS types aren't compatible and cannot be used interchangeably.

Yes, this was a known issue when TypeBox added ESM support. The initial recommendation was that if a sub dependency was based on CJS (in your case nestjs-typebox) then downstream implementations should also limit their builds to CJS (ensuring the same types are pulled in), but this was just the initial recommendation (as of 6 months ago)

I suggest to output just a single type folder to avoid this confusion.

I'm not sure I follow. TypeBox provides dual publishing for CJS and ESM which it achieves by publishing separate builds for each. Maybe there is a better way to achieve this? Ideally I would prefer to opt for ESM only (if single publishing), however limiting to ESM only would break many projects who already rely on CJS (and for historical reasons, tooling support for CJS still the most understood, meaning publishing for CJS only is likely still the most viable target for single publishing).

As far as I can tell, there does not appear to be an ideal solution for mixing and matching ESM/CJS that doesn't result in configuration pushback in user applications. But am open to reviewing the current build setup and changing it if there are better solutions available today.

Do you have any thoughts on a possible better setup?

from typebox.

akim-bow avatar akim-bow commented on July 2, 2024

@sinclairzx81 Hey, thanks for the detailed response.
In order to make myself clear, i've opened PR with my idea.
As far as i understand, having just a single type folder (but still keeping javascript files in dual build format) will solve this issue and won't break other projects bcs source files will be same and types will be CJS only, but i am almost certain that TS doesn't care which module system should be used for type resolution. Please correct me if i'm wrong.

from typebox.

akim-bow avatar akim-bow commented on July 2, 2024

However your utility for checking package integrity reports this issue.

from typebox.

akim-bow avatar akim-bow commented on July 2, 2024

There are 2 consequences following this decision listed in the link above. Probably we can check whether it's the case in the library and if so, deal with them. Removing the necessity to switch to other module system due to downstream dependency seems like a great feat to me.

from typebox.

sinclairzx81 avatar sinclairzx81 commented on July 2, 2024

@akim-bow Hi, thanks for the PR

There are 2 consequences following this decision listed in the link above. Probably we can check whether it's the case in the library and if so, deal with them. Removing the necessity to switch to other module system due to downstream dependency seems like a great feat to me.

Yes, I see what you mean by the "single types directory". This approach was actually attempted when TypeBox introduced ESM. However the "Masquerading as CJS" error would be a problem as it has a potential to break type checking in projects (which is a no go zone). The current recommendation (afaik) would be that there needs to be one set of definitions per build target (so one for CJS and one for ESM) as TypeScript will interpret each target slightly differently.


Note that the strategy TypeBox selected was largely informed by the work of the author of are-the-types-wrong (github link here) where he had done the research to verify each publishable target. TypeBox settled on the package-json-redirects strategy as it provided legacy support for Node10 (explicit sub directory resolution) as well as more modern Node resolution schemes.

Other strategies are:

So all this said, I would be open to exploring alternative builds, just so long as they aligned with attw check and satisfied the following criteria.

Requirements:

  • Node10 legacy resolution by default (if users do not use a tsconfig.json configuration)
  • Support modern ESM / CJS resolution (with tsconfig.json configuration)

This would need researching. Unfortunately the documentation for dual publishing is very light, but there may be potential to opt for one of the other build targets. It just needs looking into and ensuring the target meets the above criteria.

from typebox.

akim-bow avatar akim-bow commented on July 2, 2024

Yes, Masquerading as CJS is serious. But my question was: is it possible to restructure and rewrite the library's code in a way, that will alleviate any runtime errors? Also edge cases, when "masquerading" leads to errors, could be covered with proper tests to prevent possibility of the errors in the future.

Also i want to underline that i'm totally OK with the approach package-json-redirects. I'm just trying to change type resolution strategy still enjoying the benefits of currently settled strategy.

So all this said, I would be open to exploring alternative builds, just so long as they aligned with attw check and satisfied the following criteria.

Attw checks not really perfect bcs they don't actually analyze your codebase to say whether it will give runtime error or not. It reviews package.json files and does report a possibility for the error. It is strange that attw doesn't consider single type folder approach as a viable alternative. It is a real cases for thousands of projects. If it is crucial for your to stay in attw limits, i would like to see some motivation, just curious :)

A good example of the approach i want to achieve - @nestjs/common package. If you look at the package files, it uses single type emission. And no user has a problem with that structure judjing by open issues there.

To me, forcing user to switch to different module system, forbidding him using ESM packages is a tremendous drawback. As you can see in the top message, i wasn't doing something magical, merely tried to add your wonderful library in NestJS as validator. But probably you have other opinion regarding the seriousness of this matter?

from typebox.

akim-bow avatar akim-bow commented on July 2, 2024

I believe it is possible to make your code 100% workable in single type folder approach and eager to make some researches. Just want to make sure i don't break any of the project's principle or walking the same path twice.

from typebox.

sinclairzx81 avatar sinclairzx81 commented on July 2, 2024

@akim-bow Hi,

Hey, have closed off the PR for now as the single type directory needs additional research to go ahead. Will move this issue into discussions for now as it would be good to review the current build, but as as noted, would want to be working inline with TS and Node module resolution strategies (as working against the grain here would likely break a lot of users)

Let's keep this discussion going, as well as see if it is possible to get some additional insights from the community on this. Module systems are far too complex!

from typebox.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.