Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: watch build does not copy files on outputtarget www #4980

Closed
3 tasks done
Famoose opened this issue Oct 25, 2023 · 2 comments · Fixed by #4997
Closed
3 tasks done

bug: watch build does not copy files on outputtarget www #4980

Famoose opened this issue Oct 25, 2023 · 2 comments · Fixed by #4997
Assignees
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@Famoose
Copy link

Famoose commented Oct 25, 2023

Prerequisites

Stencil Version

4.6.0

Current Behavior

On "stencil build --dev --watch --serve" with files to copy in for example /dev folder. It does not copy them after the hot reload.

stencil.config.ts:

  {
      type: 'www',
      serviceWorker: null, // disable service workers
      copy: [{src:'dev/**/*.html', dest:'.', keepDirStructure: true}]
    },
[38:22.1]  rebuild, project, dev mode, started ...
[38:22.1]  transpile started ...
[38:22.1]  transpile finished in 3 ms
[38:22.1]  generate lazy + source maps started ...
[38:22.2]  generate lazy + source maps finished in 133 ms
[38:22.3]  rebuild finished, watching for changes... in 146 ms
<- the copy task is missing

Maybe it's a windows problem, we have spotted the error on multiple windows machines, but on Unix file system it seems to work.
We did upgrade from stencil 2.x and with 4.6 it's not working anymore. So the issue may be in previous versions too.

Expected Behavior

on file changes the file should be reflected in the /www folder

System Info

System: node 20.8.0
    Platform: windows (10.0.19045)
   CPU Model: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz (12 cpus)
    Compiler: C:\dev\projects\sten-starter\node_modules\@stencil\core\compiler\stencil.js        
       Build: 1698073283
     Stencil: 4.6.0
  TypeScript: 5.2.2
      Rollup: 2.42.3
      Parse5: 7.1.2
      Sizzle: 2.42.3
      Terser: 5.22.0

Steps to Reproduce

  • on windows checkout https://github.com/Famoose/sten-starter (its a starter project with one html file and the copy config)
  • run npm start
    Initial everything gets copied correctly
    even the watch works.
  • change something in the file.
  • watch triggers and recompile
    but the file is not updated

Code Reproduction URL

https://github.com/Famoose/sten-starter

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Oct 25, 2023
@rwaskiewicz rwaskiewicz self-assigned this Oct 25, 2023
@rwaskiewicz rwaskiewicz added the Bug: Validated This PR or Issue is verified to be a bug within Stencil label Oct 25, 2023
@ionitron-bot ionitron-bot bot removed the triage label Oct 25, 2023
rwaskiewicz added a commit that referenced this issue Nov 9, 2023
this patch ensures that paths are properly normalized by the compiler.
this ensures that regardless of the platform (operating system) that a
project is compiled on, paths are uniformly treated internally by
stencil. this has system-wide reaching effects - from the in-memory
filesystem, to configuration/output target validation, and file
generation.

previously, stencil's in-browser compilation support included a polyfill
for the following NodeJS `path` module functions: `join`, `normalize`,
`relative` & `resolve`. this polyfill did the following:
- it wrapped each of the aforementioned functions in a `normalizePath`
  function to convert Windows-style path separators (`\\`) to
  Unix/POSIX-style path separators (`/`)
- it overwrote the standard NodeJS `path` implementations for each of
  these functions.
as a result, calling `join` or any of the other three methods, even when
importing the method from `path` like below would result in the polyfill
being called:
```ts
import { join } from 'path'; // this imports the polyfilled `join`

// runs the native `path.join`, then normalizes the returned path
const filePath = join(part1, part2);
```

while this was 'nice' in that stencil engineers didn't need to think
about which implementation of `path` functions they were using, this
polyfill made some behavior of the compiler hard to understand.

the polyfills were removed in #4317 (b042d8b). this led to calls to the
aforementioned functions to call their original implementations, rather
than the wrapped implementations:
```ts
import { join } from 'path'; // imports Node's `join`

// run the native `path.join`, without any normalization
const filePath = join(part1, part2);
```

discrepencies arose where parts of the code would explicitly wrap a
call to `join()` (or one of its ilk) around a path normalization
function. this caused paths to not be uniformly normalized throughout
the codebase, leading to errors.

since the removal of in-browser compilation, additional pull requests
to fix path-related issues on windows have landed in the codebase:
- #4545 (cd58d9c)
- #4932 (b97dadc)

this commit builds on the previous commits by attempting to move
stencil's compiler completely over to polyfilled versions of the
mentioned `path` functions that are explicitly imported/called.

Some of the changes found herein were created/validated using Alice's
codemod branch, #4996
Co-authored-by: alicewriteswrongs <alicewriteswrongs@users.noreply.github.com>

STENCIL-975 Determine Scope of Path Polyfill Bug, Fix

Fixes: #4980
Fixes: #4961

Spun Off: #5036, #5032, #5029
@rwaskiewicz
Copy link
Member

@Famoose Thanks for your patience!

Can you do me a favor and give the following dev build of Stencil a try when you get a chance?

npm i @stencil/core@4.7.1-dev.1699535671.a4c3950

github-merge-queue bot pushed a commit that referenced this issue Nov 10, 2023
* fix(compiler): normalize paths on windows

this patch ensures that paths are properly normalized by the compiler.
this ensures that regardless of the platform (operating system) that a
project is compiled on, paths are uniformly treated internally by
stencil. this has system-wide reaching effects - from the in-memory
filesystem, to configuration/output target validation, and file
generation.

previously, stencil's in-browser compilation support included a polyfill
for the following NodeJS `path` module functions: `join`, `normalize`,
`relative` & `resolve`. this polyfill did the following:
- it wrapped each of the aforementioned functions in a `normalizePath`
  function to convert Windows-style path separators (`\\`) to
  Unix/POSIX-style path separators (`/`)
- it overwrote the standard NodeJS `path` implementations for each of
  these functions.
as a result, calling `join` or any of the other three methods, even when
importing the method from `path` like below would result in the polyfill
being called:
```ts
import { join } from 'path'; // this imports the polyfilled `join`

// runs the native `path.join`, then normalizes the returned path
const filePath = join(part1, part2);
```

while this was 'nice' in that stencil engineers didn't need to think
about which implementation of `path` functions they were using, this
polyfill made some behavior of the compiler hard to understand.

the polyfills were removed in #4317 (b042d8b). this led to calls to the
aforementioned functions to call their original implementations, rather
than the wrapped implementations:
```ts
import { join } from 'path'; // imports Node's `join`

// run the native `path.join`, without any normalization
const filePath = join(part1, part2);
```

discrepencies arose where parts of the code would explicitly wrap a
call to `join()` (or one of its ilk) around a path normalization
function. this caused paths to not be uniformly normalized throughout
the codebase, leading to errors.

since the removal of in-browser compilation, additional pull requests
to fix path-related issues on windows have landed in the codebase:
- #4545 (cd58d9c)
- #4932 (b97dadc)

this commit builds on the previous commits by attempting to move
stencil's compiler completely over to polyfilled versions of the
mentioned `path` functions that are explicitly imported/called.

Some of the changes found herein were created/validated using Alice's
codemod branch, #4996
Co-authored-by: alicewriteswrongs <alicewriteswrongs@users.noreply.github.com>

STENCIL-975 Determine Scope of Path Polyfill Bug, Fix

Fixes: #4980
Fixes: #4961

Spun Off: #5036, #5032, #5029

* fix validate-dist-collection tests

* fix prerendered-write-path test

* fix stencil-types tests

this commit removes a spy on `join.resolve` and updates the mocks to
properly spy on the wrapped `@utils`' `resolve` fn

* fix validate-paths tests

this commit updates the tests for `validate-paths`. it replaces
instances of `path.join` in assertions with stencil's own `join`
function. existing instances of `path.join` have been left as-is if they
pertain to user input. this allows us to exercise a user using either
path separator in CI (which runs in windows and linux).

the cache directory is now normalized if it is absolute. otherwise,
provided cache directories in `stencil.config.ts` would never get
normalized

* fix validate-output-www tests

this commit updates the tests for `validate-paths`. it replaces
instances of `path.join` in assertions with stencil's own `join`
function. existing instances of `path.join` have been left as-is if they
pertain to user input. this allows us to exercise a user using either
path separator in CI (which runs in windows and linux).

* fix validate-output-dist tests

this commit updates the tests for `validate-output`. it replaces
instances of `path.join` in assertions with stencil's own `join`
function. existing instances of `path.join` have been left as-is if they
pertain to user input. this allows us to exercise a user using either
path separator in CI (which runs in windows and linux).

this commit also switches `output-target.ts#getcomponentsDtsSrcFilePath`
over to use Stencil's join function to fix the tests

* fix validate-output-dist-custom-element tests

this commit updates the tests for `validate-output-dist-custom-element`.
it replaces instances of `path.join` in assertions with stencil's own `join
function. existing instances of `path.join` have been left as-is if they
pertain to user input. this allows us to exercise a user using either
path separator in CI (which runs in windows and linux).

* fix validate-testing tests

this commit updates the tests for `validate-testing`. it replaces
instances of `path.join` in assertions with stencil's own `join`
function. existing instances of `path.join` have been left as-is if they
pertain to user input. this allows us to exercise a user using either
path separator in CI (which runs in windows and linux).

the screenshot connector is now normalized if it is absolute. otherwise,
provided connector file path in `stencil.config.ts` would never get
normalized
@tanner-reits
Copy link
Member

The fix for this went out as a part of today's v4.7.2 release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants