[polaris-migrator] Add a migration to change inline for block comments #10749
Conversation
jbalsas
force-pushed
the
jbalsas/migration-inline-comments
branch
from
3426739 to
afcf77c
Compare
|
Didn't update the polaris docs with a reference to the migration since I'm not sure what's the strategy there, but happy to, if we think it's a good idea! |
| Comment(comment) { | ||
| comment.raws.inline = false; | ||
| comment.raws.right = ' '; | ||
| comment.raws.left = ' '; | ||
| }, |
There was a problem hiding this comment.
| Comment(comment) { | |
| comment.raws.inline = false; | |
| comment.raws.right = ' '; | |
| comment.raws.left = ' '; | |
| }, | |
| Comment(comment) { | |
| comment.raws.before = '\n'; | |
| comment.raws.inline = false; | |
| comment.raws.right = ' '; | |
| comment.raws.left = ' '; | |
| }, |
GPT suggested using comment.raws.before 🤔 . Maybe conditionally depending on the current value of comment.raws.before, let's say if it's already \n.
There was a problem hiding this comment.
Okay, so looked at this briefly yesterday and I suspect it's a bit more involved than this, because sometimes we want new break lines and some times we don't, and it depends on the prev() node.
I need to take a closer look to fine tune this, so I'll circle back to this in a few days 🤞
| '@shopify/polaris-migrator': minor | ||
| --- | ||
|
|
||
| Adds new styles-replace-inline-comments general migration |
There was a problem hiding this comment.
| Adds new styles-replace-inline-comments general migration | |
| Added `styles-replace-inline-comments` generic migration |
| 'styles-insert-stylelint-disable', | ||
| ), | ||
| 'styles-replace-custom-property': resolve('styles-replace-custom-property'), | ||
| 'styles-replace-inline-comments': resolve('styles-replace-inline-comments'), |
There was a problem hiding this comment.
| 'styles-replace-inline-comments': resolve('styles-replace-inline-comments'), |
Correct me if I'm wrong @sam-b-rose but we're not using presets right now, right? Tbh, I didn't even realize this file still existed after the consolidation of polaris-codemods and polaris-migrator.
There was a problem hiding this comment.
I left this in as a just-in-case we wanted to try to use something like @codeshift/cli; however, I think we should delete this file. I think this change safe to be included in this PR.
There was a problem hiding this comment.
Can we confirm block comments /* */ don't show up in the build (Polaris and Web)? That was the primary reason we used inline comments // for Sass migrations.
There was a problem hiding this comment.
As far as I can tell, all comments are stripped in the minify step of our build process.
| return postcss([ | ||
| stylelint({ | ||
| config: { | ||
| extends: [options.config ?? '@shopify/stylelint-polaris'], |
There was a problem hiding this comment.
Can you help me understand why we're using stylelint and @shopify/stylelint-polaris in this migration? We don't appear to be operating on the stylelint results in the migration plugin. I'm curious if we can follow the boilerplate Sass migration pattern (for example)
|
Hi! We noticed there hasn’t been activity on this PR in a while. After 30 days, it will close automatically. If it’s still relevant, or you have updates, comment and let us know. And don’t worry, you can always re-open later if needed. |
|
Going to close here since we ran this once for web and haven't needed it since. We can always come back and bring this back if needed, but probably better to keep the migrator as lean as we can to avoid bloat. |
WHY are these changes introduced?
In the context of
shopify/web's code yellow, we are looking to drop sass. The overall topic here is substraction, so we want to rewrite all comments to the CSS native block style by defaultWHAT is this pull request doing?
Creates a new migration
styles-replace-inline-commentsthat replaces inline by block commentsHow to 🎩
I think tests should be enough, but you can look at https://github.com/Shopify/web/pull/105280/commits/12c19b7f3be5f9689406ad690e368bcd55a3f894 as a reference of the output of running the codemod over a fraction of the
shopify/webcodebase🎩 checklist
README.mdwith documentation changes