Add file extensions #1781
Add file extensions #1781
Conversation
| } from './utilities/log/index.js'; | ||
| import {getErrorMarkup} from './utilities/error.js'; |
There was a problem hiding this comment.
It seems a little weird to be importing .js from a .tsx 🤔
There was a problem hiding this comment.
Ah, I see your comment here: Shopify/hydrogen#1155 (comment)
Still a bit odd.
There was a problem hiding this comment.
Agreed it's very odd. But the TS people are very adamant about it, so the industry has to live with their decision.
Builds are now passing, so that's an improvement.
|
We detected some changes in |
| // eslint-disable-next-line node/no-extraneous-import | ||
| import * as React from 'react'; | ||
|
|
||
| globalThis.React = React; |
| rules: { | ||
| // ensure that file extensions are used from now on | ||
| 'import/extensions': ['error', 'ignorePackages'], | ||
| // next two are turned off because of the conflict between TS requiring .js extensions and those .js files not actually existing in the filesystem |
There was a problem hiding this comment.
Does this also mean that erroneous imports that don't exist will not warn in the IDE?
|
I unintentionally increased the scope of this PR in the middle of it, and I believe that's now causing issues with the e2e tests that have been difficult to resolve. I think I'm going to close this PR and start over again, this time with not only a better understanding of what changes need to happen, but also understanding which changes don't need to happen. |
|
Superseded by #1857 |
Description
This is a WIP PR.
As part of #1155 , we need to move to having the full file extension in our imports.
I paused here to see what everyone's feelings were about this now that you can see the effects of it. If we feel like this is still correct, then I'll move forward and do the rest of the repo.
Additional context
Despite what this may look like, this PR doesn't change our current stance on using
index.tsfiles everywhere. However, you may notice that with this PR, it removes some of the benefits of usingindex.tsfiles, which may be another reason to remove that pattern (in the core hydrogen package, at least).Before submitting the PR, please make sure you do the following:
fixes #123)yarn changeset addif this PR cause a version bump based on Keep a Changelog and adheres to Semantic Versioning