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

Compiler APIs should allow a reload option to cache bust #5253

Closed
kitsonk opened this issue May 12, 2020 · 6 comments · Fixed by #8328
Closed

Compiler APIs should allow a reload option to cache bust #5253

kitsonk opened this issue May 12, 2020 · 6 comments · Fixed by #8328
Assignees
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)

Comments

@kitsonk
Copy link
Contributor

kitsonk commented May 12, 2020

Currently there is no way to internally cache bust with the compiler APIs, only if Deno is invoked with --reload will the compiler APIs cache bust.

@kitsonk
Copy link
Contributor Author

kitsonk commented May 12, 2020

Ref #4752

@bartlomieju
Copy link
Member

Ref #4782
Ref #4743

This is on my radar

@bartlomieju bartlomieju added cli related to cli/ dir feat new feature (which has been agreed to/accepted) labels May 21, 2020
@bartlomieju bartlomieju self-assigned this May 21, 2020
@bartlomieju bartlomieju mentioned this issue Oct 10, 2020
22 tasks
kitsonk added a commit that referenced this issue Nov 16, 2020
Fixes #4743
Closes #5253
Fixes #5631
Fixes #6116

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
Co-authored-by: Luca Casonato <lucacasonato@yahoo.com>
@TradeIdeasPhilip
Copy link

TradeIdeasPhilip commented Nov 16, 2020

I tried this with Deno 1.5.3. I can confirm that the original complaint was fixed.

However, there are a couple of new problems.

  1. The source maps are broken. When I ask for source map files, the map files are still created. However, the .js files no longer point to the map files, so I can't use the source maps.

  2. The file names changed. (I wish I could be 100% certain of the old behavior.) When I compile /home/phil/cpp_alert_server/deno/admin/ts/cvs_nq_update.ts I get file:///home/phil/cpp_alert_server/deno/admin/ts/cvs_nq_update.ts.js and file:///home/phil/cpp_alert_server/deno/admin/ts/cvs_nq_update.ts.js.map as output files. I.e. it added ".js" and ".js.map" rather than replacing ".ts" with the new extension. That's not terrible, but I would like to know the intent and if we think this is the final form.

Thanks!

EDIT: The map file is broken. Now it starts with

{"version":3,"file":"","sourceRoot":"","sources":["file:///home/phil/cpp_alert_server/deno/admin/ts/cvs_nq_update.ts"],"names":[],"mappings":"AAIA,SAAS,

I don't know what used to be in there. (I wish I'd saved a copy of the old result or the old Deno executable.) But source maps used to work in Deno 1.5.1. So they must have been relative paths. I'm running the browser on a totally different machine than the Deno server, no file sharing.

@kitsonk
Copy link
Contributor Author

kitsonk commented Nov 16, 2020

Regarding source maps, they never point at the emitted files, they point back at the source files. Yes previously they could be non-deterministic. Now they point at the absolute URI of the source. It is worth discussing more, but we would likely not address it until #4752.

Regarding the name changes, it is intentional, again, because there are all sorts of scenarios where it could be broken if we simply don't append the extension. There is no such thing as a "final form" with an unstable API. 😄

@TradeIdeasPhilip
Copy link

There is no such thing as a "final form" with an unstable API. 😄

Understood. I'm trying to decide how much to fix on my side and how much to wait for.

@TradeIdeasPhilip
Copy link

Here's my very quick and dirty work around to get source maps working again. I need to do something better, but this works for now and it explains what changed in Deno.

                    if (compiledFilePieces.isJs) {
                      body += "\n//# sourceMappingURL=" + withoutPath + ".js.map";
                    } else if (compiledFilePieces.isJsMap) {
                      body = body.replace(/(?<=\[")file:\/.*(?=\/[^\/]+.ts"\])/, ".");
                    }

In a *.ts.js file append something like //# sourceMappingURL=cvs_nq_update.js.map. In a *.ts.js.map file replace something like ["file:///home/phil/cpp_alert_server/deno/admin/ts/cvs_nq_update.ts"] with a relative URL like ["./cvs_nq_update.ts"].

jannes pushed a commit to jannes/deno that referenced this issue Dec 1, 2020
Fixes denoland#4743
Closes denoland#5253
Fixes denoland#5631
Fixes denoland#6116

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
Co-authored-by: Luca Casonato <lucacasonato@yahoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants