[Discussion] Modules that can break other modules.

[andreespirela]

Hello All. This issue is going to be a bit long as explanation is needed, but it will have a point.

On Monday, June 1 at about 5am, the code of one of my projects broke. The error was quiet interesting

error: TS2578 [ERROR]: Unused '@ts-expect-error' directive.
  // @ts-expect-error
  ~~~~~~~~~~~~~~~~~~~
    at https://deno.land/std@0.54.0/ws/mod.ts:494:3

it turns out that nowhere in my code I was importing https://deno.land/std@0.54.0/ws/mod.ts so I thought about a third-party module broke my module.
After hours investigating, it turns out that Oak has committed a change to deps.ts file that included https://deno.land/std@0.54.0/ws/mod.ts, at first I looked for the dependencies of Oak my module uses, and I made them target to oak@v5.0.0 as that version didn't have the inclusion that was breaking my code, but after that, my code continued breaking and I knew something weird was going on with a third-party module. First, I thought about it, then I was sure.

Long story short, One of my third-party modules was calling a different version of oak in relation to the version of oak my app was using, and this was breaking the code, not because of my app itself, but because of the codes that were being included in the third-party oak import.

I believe this is dangerous issue & should be addressed carefully, it requires a lot of discussion. The main points are:

1. Third-party modules can break your module easily.
   1. This is extremely unaffordable in production environments.
2. There should be a warning when a module is not imported with a target version.
3. There should be a deno flag/configuration to allow or not allow imports directly from the master branch as this would give more control to the developer over what kind of third-party libraries are being used and whether they could break the code.
4. Possibly, there should be an option to have a "force import", this would mean, if your module is forcing a version on a file from a module, and a third-party requires the same file from the same module but it is requesting the master branch, then through the "force import", Deno would know that you have already specified a version to that file of that module and thus that one should be the file to be used for all the modules (inside your app or third-party) that require it.
   1. This would give more flexibility to the developer on what kind of imports to use, as if the code breaks, the developer already knows that there is a conflict of versions and it is the developer's fault & responsibility to get the work around it.

This is something that requires a lot of discussion, but it is definitely something that needs to be addressed. There is a clear example of this. My application broke for over 10 hours, and this could have happened in pre-production environments, even worse, some scenarios can make this happen in production. This has been addressed several times I believe, but as of right now, nothing clear.

Thank you for reading,
I can't wait to read your opinion on this and your possible solutions.

Regards,
Juan.

CC: @lucacasonato

[nayeemrmn]

Of course a third party module can break if you update it... what do you mean? Are you saying it somehow broke without you updating anything in your own deps.ts?

[andreespirela]

Correct. My code was targeting oak@v5.0.0, a third-party module I use was targeting oak@master . oak@master was breaking my code. At plain sight you can’t tell what’s happening and have to go deep & deeper because if a third party is importing something that’s not “compatible” with your app you won’t know until checking your cache folder or checking manually repo after repo (module after module)

[nayeemrmn]

Ah I see, you used a locked version of a module that used an unlocked version of oak. This isn't really an earth shattering issue with Deno, the module you're using is faulty because it does this. Kind of like a module that is coded to throw a random error after a certain date, which it may as well be. Raise a bug report with them or use something else.

You can catch these situations properly using lock files. There are couple of ways to recover from them:

• Use an import map to override that module's oak@master import.
• Use this pattern so the expected source is always available and being used.

[kitsonk]

And if you are ever unsure of what your dependencies are:

$ deno info my_file.ts

[lucacasonato]

I think there is a point to be made here that we can improve the semantics around importing implicit master for deno.land/x. I propose the following:

1. Use the existing X-Deno-Warning infrastructure to add a warning to all requests that use an implicit version, not just std.
2. Add a "implicit_default": "warn" key in the database.json so every module can choose for itself if implicit versions should warn or not. This could later even be extended to a "implicit_default": false which would disable implicit version importing all together.
3. Add a link to deno.land/x in the warning to where people can find out more about why an implicit default is bad.
4. Add a import trace to the warning in Deno.

WDYT?

[andreespirela]

I think that fits in well in what is needed. I agree there should be more official perspective from Deno that importing from master is a bad practice (possible but bad) so there should be warnings, a page explaining that. I guess all that could be done without too much effort & in a useful way.
Let’s see where this discussion gets...

[jsejcksn]

It could be helpful for the module browser of /x to return the latest tag of each module instead of master (and return master as a fallback in case the module doesn't use tags).

A more extreme position would be to disallow modules which don't utilize [semver] tags.

[lucacasonato]

We are working on this. In the very near future this problem will be solved.

[jsejcksn]

We are working on this. In the very near future this problem will be solved.

@lucacasonato Any details to share? A link, etc.?

[lucacasonato]

Update: deno.land/x/MODULE/path.ts now redirects to the latest tag rather than default branch. We might restrict this more in the future. All other paths are now unchanging. Read more at https://deno.land/posts/registry2

[jsejcksn]

@lucacasonato I noticed that you said release here, but tags are referenced in the link you shared. It is an important distinction. Will you clarify the behavior? Does /x link only to release tags or to all tags?

[lucacasonato]

Sorry, I meant to say all tags.

[jsejcksn]

@lucacasonato Was there discussion around this happening somewhere you can share a link to?

[nayeemrmn]

@jsejcksn There's some discussion here: denoland/deno#6723 (comment).

[n10000k]

I still think this is a good point, I know there’s still modules which are supporting the first version of deno and using the old std versions in some of the most used modules.

I know the cdn is now immutable but what’s stopping modules being released and never updated in the future. How would this be moderated also.

Some points:

• How do module maintainers keep track of dependency versions?
• At what point / criteria should a module be removed from the cdn due to not being maintained or kept up to date;
• Security, how do you mark a version being a security risk to the end users?

An idea: A github bot;

• All third party modules require the github bot to be enabled;
• It works like dependabot, highlighting out of date versions etc, @hayd has his concept already working for deps scanning;
• History scanning if the bot detects it’s not been updated for a certain period of time it can flag the module;
• Security warnings, if the module holds a version with a security flaw dependency it can mark it on the cdn and add a security flaw header to anyone who pulls that module will get an alert;

A github bot like - https://github.com/probot/probot

Just an idea, feedback?

[jsejcksn]

I think some of these are great considerations.

The core nature of some modules are such that maintenance is only rarely or never required. Just because a module repo lacks regular commits doesn't necessarily mean its code is outdated. The concept of stale or unmaintained are very contextual, so I think that aspect will need careful consideration if implemented.

[n10000k]

Correct It would need to criteria to determine if it's stale or unmaintained.

• Been recommended updates over a period of time by the bot and not been updated, header for deprecate?
• A downvote and upvote system maybe, with a calculation over a period of time it gets flagged for review

I think a bot would be a cool, automated way to go would just keep checking module repos. However the factors into this are obviously, cost, scalability etc.

[brandonkal]

@narwy and others: I would suggest bringing https://web.crev.dev/ to Deno. Using that system, @andreespirela could have posted a quick review alerting other users about the broken dependency version. Users can then easily see if there is any reported issues with their dependencies and choose which reviewers to trust. A bot could easily fit into that, though I suspect it would generate a lot of noise. i.e. the stale bot is the worst thing to hit GitHub. Bots can be useful but can only go so far. So I would say the first step would be to integrate a system in which humans can also review dependencies.

[crowlKats]

We reccommend using lockfiles to avoid such odd cases breaking existing code, and this isnt directly actionable by us in either form of a feature of the CLI or change in the registry.