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.

Sign up for GitHub

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

Jump to bottom

BREAKING: fix(Worker): Disable relative module specifiers #5266

Merged
merged 4 commits into from
Jun 9, 2020
Merged

BREAKING: fix(Worker): Disable relative module specifiers #5266

merged 4 commits into from
Jun 9, 2020

Conversation

nayeemrmn
Copy link
Collaborator

bartlomieju
bartlomieju reviewed May 13, 2020
View reviewed changes
cli/ops/worker_host.rs Outdated Show resolved Hide resolved
@bartlomieju bartlomieju added the cli related to cli/ dir label May 15, 2020
@nayeemrmn
Copy link
Collaborator Author

@bartlomieju Please review. Based on stability promises for web APIs, this can be minorly released?

@bartlomieju
Copy link
Member

Based on stability promises for web APIs, this can be minorly released?

Yes, it can. I'll review in the coming days - just want to cool off cli changes for a few days :)

@nayeemrmn nayeemrmn force-pushed the worker-relative-import branch from e86a491 to 639a042 Compare May 21, 2020 12:35
@bartlomieju bartlomieju added this to the 1.1.0 milestone May 21, 2020
@nayeemrmn nayeemrmn changed the base branch from master to v1.1 May 22, 2020 12:59
@nayeemrmn nayeemrmn force-pushed the worker-relative-import branch from 639a042 to 8008ef8 Compare May 22, 2020 12:59
@ry ry closed this Jun 6, 2020
@bartlomieju bartlomieju reopened this Jun 6, 2020
@nayeemrmn nayeemrmn changed the base branch from v1.1 to master June 8, 2020 16:32
@nayeemrmn nayeemrmn force-pushed the worker-relative-import branch from 8008ef8 to 1526561 Compare June 8, 2020 16:32
bartlomieju
bartlomieju approved these changes Jun 9, 2020
View reviewed changes
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @nayeemrmn

@bartlomieju bartlomieju merged commit 44251ce into denoland:master Jun 9, 2020
@nayeemrmn nayeemrmn deleted the worker-relative-import branch June 9, 2020 14:37
@wongjiahau
Copy link
Contributor

@nayeemrmn will this approach be broken on Windows?

@nayeemrmn
Copy link
Collaborator Author

@nayeemrmn will this approach be broken on Windows?

You mean how new URL(...).pathname is broken on Windows? No. The worker script takes a module specifier so we don't give it a path, we pass new URL(...).href as you can see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartlomieju bartlomieju bartlomieju approved these changes

Assignees
No one assigned
Labels
cli related to cli/ dir
Projects
None yet
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.

Worker: Disable relative import specifiers
4 participants
@nayeemrmn @bartlomieju @wongjiahau @ry