fix(scripts): fix Esbuild scripts to allow to run on Windows #5930
Conversation
christian-bromann
force-pushed
the
cb/fix-windows-build
branch
8 times, most recently
from
b3fb8c9 to
dbad1ef
Compare
|
Note: the pending jobs should properly resolve once merged into |
| '@stencil/core/cli': './cli/index.js', | ||
| '@sys-api-node': './sys/node/index.js', | ||
|
|
||
| // mapping node.js module names to `sys/node` "cache" | ||
| // | ||
| // these allow us to bundle and ship a dependency (like `glob`) as part | ||
| // of the Stencil distributable but also have our separate bundles | ||
| // reference the same file | ||
| glob: './sys/node/glob.js', |
There was a problem hiding this comment.
What's the reason for dropping these? Just not needed in all instances of getEsbuildAliases() being called?
There was a problem hiding this comment.
Correct, some aliases are only needed in certain components e.g. glob which is why I removed it from here. The alias really depends on where the bundled file is located, e.g. how many levels they are nested within Stencil. Most artifacts are located within a directory, e.g. /testing/index.js or /compiler/stencil.js so I kept some global ones but made sure the packages that are located in a more deeper directory structure receive a custom alias.
| const sysNodeAliases = getEsbuildAliases(); | ||
| const sysNodeAliases = { | ||
| ...getEsbuildAliases(), | ||
| '@stencil/core/compiler': '../../compiler/stencil.js', |
There was a problem hiding this comment.
If this path is different than what's spit out by getEsbuildAliases(), does it make sense to have that function generate the path based on some directory? Why are they different in the first place?
There was a problem hiding this comment.
there are some build artifacts that located multiple folders deep that require a different alias path, e.g. for /testing/index.js the alias for @stencil/core/compiler has to be ../compiler/stencil.js while for /sys/node/index.js it has to be ../../compiler/stencil.js.
There was a problem hiding this comment.
Right, so I was thinking we could figure out how deeply nested we are based on the relative path from __filename to src or something and use that info to generate the relative paths.
Or, alternatively, remove the helper altogether. What you have works, but feels a bit odd that we use it and then override the returned aliases in quite a few spots.
Open to thoughts!
There was a problem hiding this comment.
Most Stencil components can be successfully compiled using the common defined aliases as implemented in scripts/esbuild/utils/index.ts, there are only two artifacts that require modifications to these aliases which is:
- one artifact in
scripts/esbuild/sys-node.ts - and
scripts/esbuild/internal-platform-testing.ts
If you want I can get rid of the getEsbuildAliases function and have this defined per component but I think this will create more code than we have right now.
Wdyt?
There was a problem hiding this comment.
I think it's fine for now if it's only a couple spots. Thanks for the pushback, no need to overcomplicate
christian-bromann
force-pushed
the
cb/fix-windows-build
branch
from
663118d to
d2f2e75
Compare
christian-bromann
force-pushed
the
cb/fix-windows-build
branch
from
d2f2e75 to
c892100
Compare
|
Released in Stencil |
What is the current behavior?
Stencil fails when trying to build it on a Windows machine.
STENCIL-1369
What is the new behavior?
It turns out that the alias path needed to get normalized allowing Esbuild to pick up the right file and acknowledge it as external dependency.
This patch will also make some tweaks to the pipeline to run the
buildandlintertasks first and then run everything else. Before we were running WebdriverIO immediately.Documentation
Does this introduce a breaking change?
Testing
Updated our pipeline to have the project being build on Windows so we ensure we don't introduce regressions.
Other information
n/a