Rewrite [resolution] dependencies from lockfile version #1693
Conversation
🦋 Changeset detectedLatest commit: a8f0a22 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
cristiano-belloni
changed the title
| }), | ||
| {}, | ||
| ); | ||
| .replace('[version]', version || externalResolutions[name]) |
There was a problem hiding this comment.
Is there a case where version would not be defined?
There was a problem hiding this comment.
Is there a case where version would not be defined?
Good question. There is at the moment, because we relaxed our criteria to only warn in case a version is nor present in (on e of the) package.json. The reason is that some widely used libraries rely on transitive dependencies (dm me for more info)
| createRewriteDependenciesPlugin( | ||
| { | ||
| ...dependencies, | ||
| 'react-dom': dependencies.react, |
There was a problem hiding this comment.
Surely react-dom must be a dependency in the tree for starting a view to work, so we should use the resolution version from the repo in the case where react-dom is a different version to react?
| @@ -11,10 +11,21 @@ import type { Dependency } from '@schemastore/package'; | |||
|
|
|||
| type FileType = '.css' | '.js'; | |||
|
|
|||
| export const indexFile = ` | |||
There was a problem hiding this comment.
Seems a bit non-descript. Is this not DEFAULT_ESM_VIEW_INDEX_TEMPLATE?
| const lockDeps = await buildDepTree( | ||
| // Build a dependency tree from the lockfile, using target dependencies and root dependencies in order of specificity | ||
| JSON.stringify( | ||
| Object.assign(Object.create(null), targetManifest, { |
There was a problem hiding this comment.
Maybe object { ...spread } might be easier to read here?
There was a problem hiding this comment.
This is following a comment by @steveukx - there might be dependencies called as object default members.
There was a problem hiding this comment.
This one would be fine as a { ...spread } - it wasn't in the other case because the resulting object was used with a if (key in object) check without an object.hasOwnProperty(key) filter.
There was a problem hiding this comment.
This one would be fine as a { ...spread } - it wasn't in the other case because the resulting object was used with a if (key in object) check without an object.hasOwnProperty(key) filter.
This PR adds the
[resolution]tag in the CDN template to point to the pinned version as resolved fromyarn.lock. Example (assuming apackage.jsonentry of"react": "^17.0.0"and ayarn.lockresolution of17.0.2):EXTERNAL_CDN_TEMPLATE"https://cdn.skypack.dev/[name]@[version]"import React from "https://cdn.skypack.dev/react@^17.0.0""https://cdn.skypack.dev/[name]@[resolution]"import React from "https://cdn.skypack.dev/react@17.0.2"