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

feat(alias loader): drop package.json support #35

Merged
merged 5 commits into from
Nov 20, 2024
Merged

Conversation

AugustinMauroy
Copy link
Collaborator

@AugustinMauroy AugustinMauroy commented Nov 15, 2024

Resolves #20

Copy link
Owner

@JakobJingleheimer JakobJingleheimer left a comment

Choose a reason for hiding this comment

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

The README still needs to be updated to mention package.json imports

packages/alias/alias.mjs Outdated Show resolved Hide resolved

Choose a reason for hiding this comment

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

I think this loader should be marked @deprecated with a link to the readme explaining to use pjson imports

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not fan of that because this ts feature isn't deprecated. So +1 to invite user to go on subpath but still usefull for some user.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also not sure if web pack and other handle sub path for front end code. Because aliases can be used in react app.

Copy link
Owner

@JakobJingleheimer JakobJingleheimer Nov 15, 2024

Choose a reason for hiding this comment

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

tsconfig "paths" is an abortion of a feature (my stackoverflow post "Why are these tsconfig paths not working?" has like a million hits) that has plagued the ecosystem for neigh a decade. I think we should do everything we can to encourage users against it. I think marking it deprecated is appropriate because it should be removed as soon as possible: subpath imports is a far better option.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suggest we leave it like that here. Because it will really help people (example nodejs.org 😅). And we resume the discussion on nodejs with a codemod in our hands

packages/alias/alias.spec.mjs Outdated Show resolved Hide resolved
@AugustinMauroy
Copy link
Collaborator Author

The README still needs to be updated to mention package.json imports

The README didn't mention support of package.json
Capture d’écran 2024-11-15 à 22 23 54

@JakobJingleheimer
Copy link
Owner

The README still needs to be updated to mention package.json imports

The README didn't mention support of package.json

Correct. It should.

Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
JakobJingleheimer and others added 2 commits November 20, 2024 16:58
Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
Copy link
Owner

@JakobJingleheimer JakobJingleheimer left a comment

Choose a reason for hiding this comment

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

🙌 🚀

@JakobJingleheimer JakobJingleheimer merged commit 228df77 into main Nov 20, 2024
2 of 4 checks passed
@JakobJingleheimer JakobJingleheimer deleted the update-alias branch November 20, 2024 23:09
@JakobJingleheimer JakobJingleheimer changed the title feat(aliase): drop support package.json feat(alias loader): drop package.json support Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove alias loader support for pjson.alias a& recommend pjson.imports
2 participants