Skip to content

Commit

Permalink
Fix external-all(*) overrides non-external module (#873)
Browse files Browse the repository at this point in the history
* wip

* wip

* Rewrite `encodeBuildArgs` function

* Update testings

* Move testings

* Revert "Move testings"

This reverts commit 04557f7.

* Move testings
  • Loading branch information
ije committed Sep 2, 2024
1 parent 1071d7a commit cf70f37
Show file tree
Hide file tree
Showing 52 changed files with 388 additions and 426 deletions.
22 changes: 13 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,27 @@ With [import maps](https://github.com/WICG/import-maps), you can even use bare i
- **[NPM](https://npmjs.com)**:
```js
// Examples
import React from "https://esm.sh/react"; // 18.3.0 (latest)
import React from "https://esm.sh/react"; // latest
import React from "https://esm.sh/react@17"; // 17.0.2
import React from "https://esm.sh/react@beta"; // 19.0.0-beta-94eed63c49-20240425
import { renderToString } from "https://esm.sh/react-dom@18.3.0/server"; // submodules
import React from "https://esm.sh/react@beta"; // latest beta
import { renderToString } from "https://esm.sh/react-dom/server"; // sub-modules
```
- **[JSR](https://jsr.io)** (starts with `/jsr/`):
```js
// Examples
import { encodeBase64 } from "https://esm.sh/jsr/@std/encoding@1.0.0/base64";
import { Hono } from "https://esm.sh/jsr/@hono/hono@4";
```
- **[GitHub](https://github.com)** (starts with `/gh/`):
```js
// Examples
import tslib from "https://esm.sh/gh/microsoft/tslib@2.6.0"; // '2.6.0' is the git tag
// or fetch an asset file from github
fetch("https://esm.sh/gh/microsoft/fluentui-emoji/assets/Party%20popper/Color/party_popper_color.svg");
import tslib from "https://esm.sh/gh/microsoft/tslib"; // latest
import tslib from "https://esm.sh/gh/microsoft/tslib@2.6.0"; // the version '2.6.0' is a git tag
```
- **[JSR](https://jsr.io)** (starts with `/jsr/`):
- **[pkg.pr.new](https://pkg.pr.new)** (starts with `/pr/`):
```js
// Examples
import { encodeBase64 } from "https://esm.sh/jsr/@std/encoding@1.0.0/base64";
import { Hono } from "https://esm.sh/jsr/@hono/hono@4";
import { Bench } from "https://esm.sh/pr/tinylibs/tinybench/tinybench@a832a55";
```

### Specifying Dependencies
Expand Down
28 changes: 14 additions & 14 deletions server/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,14 @@ func (ctx *BuildContext) Build() (ret BuildResult, err error) {
return ctx.buildTypes()
}

// query the build result from db
// query previous build
ret, ok := ctx.Query()
if ok {
return
}

// check if the package is deprecated
if ctx.isDeprecated == "" && !ctx.module.FromGithub && !strings.HasPrefix(ctx.module.PkgName, "@jsr/") {
if ctx.isDeprecated == "" && !ctx.module.GhPrefix && !strings.HasPrefix(ctx.module.PkgName, "@jsr/") {
var info PackageJSON
info, err = ctx.npmrc.fetchPackageInfo(ctx.module.PkgName, ctx.module.PkgVersion)
if err != nil {
Expand Down Expand Up @@ -196,9 +196,9 @@ func (ctx *BuildContext) install() (err error) {

func (ctx *BuildContext) buildModule() (result BuildResult, err error) {
// build json
if strings.HasSuffix(ctx.module.SubModule, ".json") {
if strings.HasSuffix(ctx.module.SubModuleName, ".json") {
nmDir := path.Join(ctx.wd, "node_modules")
jsonPath := path.Join(nmDir, ctx.module.PkgName, ctx.module.SubModule)
jsonPath := path.Join(nmDir, ctx.module.PkgName, ctx.module.SubModuleName)
if existsFile(jsonPath) {
var jsonData []byte
jsonData, err = os.ReadFile(jsonPath)
Expand All @@ -220,7 +220,7 @@ func (ctx *BuildContext) buildModule() (result BuildResult, err error) {

entry := ctx.resolveEntry(ctx.module)
if entry.isEmpty() {
err = fmt.Errorf("could not resolve entry")
err = fmt.Errorf("could not resolve build entry")
return
}
log.Debugf("build(%s): Entry%+v", ctx.module, entry)
Expand Down Expand Up @@ -257,7 +257,7 @@ func (ctx *BuildContext) buildModule() (result BuildResult, err error) {
if err != nil {
return
}
importUrl := ctx.getImportPath(mod, ctx.getBuildArgsPrefix(mod, false))
importUrl := ctx.getImportPath(mod, ctx.getBuildArgsPrefix(false))
buf := bytes.NewBuffer(nil)
fmt.Fprintf(buf, `export * from "%s";`, importUrl)
if r.HasDefaultExport {
Expand All @@ -276,8 +276,8 @@ func (ctx *BuildContext) buildModule() (result BuildResult, err error) {
var input *api.StdinOptions

entryModuleSpecifier := ctx.module.PkgName
if ctx.module.SubModule != "" {
entryModuleSpecifier += "/" + ctx.module.SubModule
if ctx.module.SubModuleName != "" {
entryModuleSpecifier += "/" + ctx.module.SubModuleName
}

if entry.esm == "" {
Expand Down Expand Up @@ -513,7 +513,7 @@ func (ctx *BuildContext) buildModule() (result BuildResult, err error) {

// externalize the _parent_ module
// e.g. "react/jsx-runtime" imports "react"
if ctx.module.SubModule != "" && specifier == ctx.module.PkgName && ctx.bundleMode != BundleAll {
if ctx.module.SubModuleName != "" && specifier == ctx.module.PkgName && ctx.bundleMode != BundleAll {
externalPath, err := ctx.resolveExternalModule(ctx.module.PkgName, args.Kind)
if err != nil {
return api.OnResolveResult{}, err
Expand Down Expand Up @@ -578,10 +578,10 @@ func (ctx *BuildContext) buildModule() (result BuildResult, err error) {
if path.Ext(fullFilepath) == "" || !existsFile(fullFilepath) {
subPath := utils.CleanPath(moduleSpecifier)[1:]
entry := ctx.resolveEntry(Module{
PkgName: ctx.module.PkgName,
PkgVersion: ctx.module.PkgVersion,
SubModule: toModuleBareName(subPath, true),
SubPath: subPath,
PkgName: ctx.module.PkgName,
PkgVersion: ctx.module.PkgVersion,
SubModuleName: toModuleBareName(subPath, true),
SubPath: subPath,
})
if args.Kind == api.ResolveJSImportStatement || args.Kind == api.ResolveJSDynamicImport {
if entry.esm != "" {
Expand Down Expand Up @@ -1290,7 +1290,7 @@ func (ctx *BuildContext) buildTypes() (ret BuildResult, err error) {

func (ctx *BuildContext) transformDTS(types string) (err error) {
start := time.Now()
buildArgsPrefix := ctx.getBuildArgsPrefix(ctx.module, true)
buildArgsPrefix := ctx.getBuildArgsPrefix(true)
n, err := transformDTS(ctx, types, buildArgsPrefix, nil)
if err != nil {
return
Expand Down
30 changes: 14 additions & 16 deletions server/build_args.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func decodeBuildArgs(npmrc *NpmRC, argsString string) (args BuildArgs, err error
} else if strings.HasPrefix(p, "c") {
args.conditions = append(args.conditions, strings.Split(p[1:], ",")...)
} else if strings.HasPrefix(p, "x") {
p, _, _, _, e := validateModulePath(npmrc, p[1:])
p, _, _, _, e := praseESMPath(npmrc, p[1:])
if e == nil {
args.jsxRuntime = &p
}
Expand All @@ -80,14 +80,12 @@ func decodeBuildArgs(npmrc *NpmRC, argsString string) (args BuildArgs, err error
return
}

func encodeBuildArgs(args BuildArgs, pkg Module, isDts bool) string {
func encodeBuildArgs(args BuildArgs, isDts bool) string {
lines := []string{}
if len(args.alias) > 0 {
var ss sort.StringSlice
for from, to := range args.alias {
if from != pkg.PkgName {
ss = append(ss, fmt.Sprintf("%s:%s", from, to))
}
ss = append(ss, fmt.Sprintf("%s:%s", from, to))
}
if len(ss) > 0 {
ss.Sort()
Expand All @@ -97,9 +95,7 @@ func encodeBuildArgs(args BuildArgs, pkg Module, isDts bool) string {
if len(args.deps) > 0 {
var ss sort.StringSlice
for name, version := range args.deps {
if name != pkg.PkgName {
ss = append(ss, fmt.Sprintf("%s@%s", name, version))
}
ss = append(ss, fmt.Sprintf("%s@%s", name, version))
}
if len(ss) > 0 {
ss.Sort()
Expand All @@ -112,9 +108,7 @@ func encodeBuildArgs(args BuildArgs, pkg Module, isDts bool) string {
if args.external.Len() > 0 {
var ss sort.StringSlice
for _, name := range args.external.Values() {
if name != pkg.PkgName {
ss = append(ss, name)
}
ss = append(ss, name)
}
if len(ss) > 0 {
ss.Sort()
Expand Down Expand Up @@ -189,23 +183,27 @@ func fixBuildArgs(npmrc *NpmRC, installDir string, args *BuildArgs, module Modul
args.alias = alias
}
if len(args.deps) > 0 {
newDeps := map[string]string{}
deps := map[string]string{}
for name, version := range args.deps {
if module.PkgName == "react-dom" && name == "react" {
// react version should be the same as react-dom
continue
}
if depsSet.Has(name) {
newDeps[name] = version
if name != module.PkgName {
deps[name] = version
}
}
}
args.deps = newDeps
args.deps = deps
}
if args.external.Len() > 0 {
external := NewStringSet()
for _, name := range args.external.Values() {
if strings.HasPrefix(name, "node:") || depsSet.Has(name) {
external.Add(name)
if name != module.PkgName || module.SubPath != "" {
external.Add(name)
}
}
}
args.external = external
Expand All @@ -226,7 +224,7 @@ func walkDeps(npmrc *NpmRC, installDir string, module Module, mark *StringSet) (
}
if existsFile(pkgJsonPath) {
err = utils.ParseJSONFile(pkgJsonPath, &p)
} else if regexpFullVersion.MatchString(module.PkgVersion) || module.FromGithub {
} else if regexpFullVersion.MatchString(module.PkgVersion) || module.GhPrefix {
p, err = npmrc.installPackage(module)
} else {
return nil
Expand Down
8 changes: 3 additions & 5 deletions server/build_args_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ func TestEncodeBuildArgs(t *testing.T) {
BuildArgs{
alias: map[string]string{"a": "b"},
deps: map[string]string{
"c": "1.0.0",
"d": "1.0.0",
"e": "1.0.0",
"foo": "1.0.0", // to be ignored
"c": "1.0.0",
"d": "1.0.0",
"e": "1.0.0",
},
external: external,
exports: exports,
Expand All @@ -29,7 +28,6 @@ func TestEncodeBuildArgs(t *testing.T) {
keepNames: true,
ignoreAnnotations: true,
},
Module{PkgName: "foo"},
false,
)
args, err := decodeBuildArgs(nil, buildArgsString)
Expand Down
Loading

0 comments on commit cf70f37

Please sign in to comment.