search

elirasza / stylelint-stylistic

Plugin for endangered stylelint stylistic rules.
Other
64 stars 3 forks source link

tsconfig.json: set target to ES2020 #7

Closed XhmikosR closed 1 year ago

XhmikosR commented 1 year ago

Even 2020 might be low for Node.js 14, but it surely is better than ES5 :)

BTW PRs should run CI. NVM they do now :)

XhmikosR commented 1 year ago

Another thing, maybe sourcemaps for production are moot?

We now have as many files you ship sourcemaps + types.

elirasza commented 1 year ago

I've read some articles about the subject, it seems that in our particular case, source maps would be useless because there is no realistic use-case where someone would like to debug this plugin from his / her codebase.

In the actual state of the code, they are non-practical anyway because they point to a relative path that does not exist in the published package.

So if this use-case happens to exist, there should be two changes to make :

Do you think this is too much of a hassle, that source maps should be removed ? Or this compromise seems acceptable ?

XhmikosR commented 1 year ago

I think sourcemaps should be disabled for production but perhaps keep them only for development with a second tsconfig, if you think they are useful.

Generally, the less the files a package has the better :)

elirasza commented 1 year ago

Your reasoning is good to me ! Instead of a a second tsconfig, I have added a new build:production npm script to opt-in for source maps removal. This script is the one which will be run in the release action, thus pushed to npm. The old build script will work the same as before, with the exception the JS source maps are now inline.

See https://github.com/elirasza/stylelint-stylistic/commit/d2eb6aa642c8f6ed0252fb970a7cd46ea282791e for complete commit.

elirasza commented 1 year ago

BTW, do you know why the PR is not considered as merged ? Your commit has been rebased on main.

XhmikosR commented 1 year ago

You merge after cherry picking? The hash must be different that's why.

In such cases change the commit message to read "closes PR number here".

On Tue, Apr 4, 2023, 19:30 Elirasza @.***> wrote:

BTW, do you know why the PR is not considered as merged ? Your commit has been rebased on main.

— Reply to this email directly, view it on GitHub https://github.com/elirasza/stylelint-stylistic/pull/7#issuecomment-1496271940, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACVLNNVLWDDZDAEGKBIJ5LW7REB5ANCNFSM6AAAAAAWRWYZSU . You are receiving this because you authored the thread.Message ID: @.***>