stackblitz / tutorialkit

TutorialKit by StackBlitz - Create interactive tutorials powered by the WebContainer API
https://tutorialkit.dev
MIT License
232 stars 22 forks source link

chore: rename import `@tutorialkit/test-utils` to `test-utils` #273

Open Nemikolh opened 2 weeks ago

Nemikolh commented 2 weeks ago

This PR reverts the renaming of the import back to how it was: test-utils (was renamed to @tutorialkit/test-utils in #141).

This change aligns better with conventions we have in other repositories and also makes it easier to find what the import correspond to given the name of the package is test-utils and not @tutorialkit/test-utils. Note that we could instead change that package name to be @tutorialkit/test-utils but this is not a desirable change we want given it's not meant to be published.

stackblitz[bot] commented 2 weeks ago

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

AriPerkkio commented 2 weeks ago

Internal packages should always use scoped package names. This is a security feature.

But if that convention hasn't been followed elsewhere, let's not follow it here either.

Nemikolh commented 2 weeks ago

@AriPerkkio How is it a security feature?

AriPerkkio commented 2 weeks ago
- import { resetProcessFactory, setProcessFactory } from '@tutorialkit/test-utils';
+ import { resetProcessFactory, setProcessFactory } from 'test-utils';

This could import test-utils npm package, not our own internal one. In larger organizations it's quite common that all internal packages are required to be scoped ones, so that it's easy to detect which ones can be trusted. This post might describe the idea better: https://github.blog/security/supply-chain-security/avoiding-npm-substitution-attacks/#tldr

But well, if this kind of convention isn't followed in other packages, it's fine to not follow it here either.

AriPerkkio commented 2 weeks ago

I recommend to read this one as well if you haven't seen it earlier: https://medium.com/@alex.birsan/dependency-confusion-4a5d60fec610 It's a bit longer post but really good. 💯

Nemikolh commented 2 weeks ago

Oh thanks for the pointers! Maybe we could use an import path that cannot be published then like starting with ~ or something. Let's discuss it with everyone else on Monday then.

I don't like using the @tutorialkit prefix because it's confusing as where this is coming from and I don't want the package to be accidentally published.

Nemikolh commented 2 weeks ago

@AriPerkkio I finished reading both, thanks again for sharing! 🤩

The behaviour of --extra-index-url in python is wild 😱

AriPerkkio commented 2 weeks ago

like starting with ~ or something.

That would work. I've seen ~, @/ and # prefixes used a lot for this. The # seems to be related to sub path imports but I haven't used that before. 🤔